David Howells wrote:
> Rearrange the constituent bits of i_flags to be consistent with the flag values
> returned by the FS_IOC_GETFLAGS ioctl where flags with common meanings occur.
> Otherwise pack the bits of i_flags as low as possible so that RISC CPUs can
> use small instructions.
>
> This allows those filesystems that use i_flags (Ext2/3/4) to simplify their
> get/set flags routines.
>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
Seems like a good idea to me, although:
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 3675088..da5fd2d 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1259,38 +1259,17 @@ Egdp:
>
> void ext2_set_inode_flags(struct inode *inode)
> {
> - unsigned int flags = EXT2_I(inode)->i_flags;
> -
> - inode->i_flags &= ~(S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC);
> - if (flags & EXT2_SYNC_FL)
> - inode->i_flags |= S_SYNC;
> - if (flags & EXT2_APPEND_FL)
> - inode->i_flags |= S_APPEND;
> - if (flags & EXT2_IMMUTABLE_FL)
> - inode->i_flags |= S_IMMUTABLE;
> - if (flags & EXT2_NOATIME_FL)
> - inode->i_flags |= S_NOATIME;
> - if (flags & EXT2_DIRSYNC_FL)
> - inode->i_flags |= S_DIRSYNC;
Maybe a stern comment should be here about the relationship between
this and the flag definitions...
> + unsigned int flags = EXT2_I(inode)->i_flags & S_FS_IOC_FLAGS;
> +
> + inode->i_flags = (inode->i_flags & ~S_FS_IOC_FLAGS) | flags;
> }
>
...
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 48616db..5808b9b 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
...
> +/*
> + * Inode flags - they have no relation to superblock flags now
> + *
> + * Where applicable, flags that are also in FS_IOC_GETFLAGS have the same bit
> + * position
> + */
... a stern comment here about not re-ordering since other fs/*/* code depends
strongly on this order?
"where applicable" sounds like a wish, but now these can't be changed, right?
If somebody rearranges these bad things will happen.
-Eric
> +#define S_DEAD 0x00000001 /* removed, but still open directory */
> +#define S_NOQUOTA 0x00000002 /* Inode is not counted to quota */
> +#define S_NOCMTIME 0x00000004 /* Do not update file c/mtime */
> +#define S_SYNC 0x00000008 /* [FS_SYNC_FL] Writes are synced at once */
> +#define S_IMMUTABLE 0x00000010 /* [FS_IMMUTABLE_FL] Immutable file */
> +#define S_APPEND 0x00000020 /* [FS_APPEND_FL] Append-only file */
> +#define S_SWAPFILE 0x00000040 /* Do not truncate: swapon got its bmaps */
> +#define S_NOATIME 0x00000080 /* [FS_NOATIME_FL] Do not update access times */
> +#define S_PRIVATE 0x00000100 /* Inode is fs-internal */
> +#define S_DIRSYNC 0x00010000 /* [FS_DIRSYNC_FL] Directory mods are synchronous */
> +
> +#define S_FS_IOC_FLAGS 0x000100B8 /* the S_xxx flags that are also FS_xxx_FL flags */
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
