This is my first patch to ext4 and the kernel. It's simple, but you
might give it extra scrutiny since I'm new and all.
I'm working off the dev branch of git://git.kernel.org/pub/scm/linux/
kernel/git/tytso/ext4.git. Let me know if that's the wrong place to be.
Thanks,
Tyson
Tyson Nottingham (3):
ext4: move inode flags enum in preparation for cleanup
ext4: define inode flags in terms of enum values
ext4: define visible/modifiable flag masks in terms of inode flags
fs/ext4/ext4.h | 210 +++++++++++++++++++++++++++-----------------------------
fs/ext4/super.c | 3 -
2 files changed, 103 insertions(+), 110 deletions(-)
--
2.7.4
Define inode flag bit masks in terms of inode flag enum values to make
relationship between them clearer. Also, make the masks unsigned longs
instead of ints (as a consequence of the way they are now defined).
Signed-off-by: Tyson Nottingham <[email protected]>
---
This change introduces some long lines. It's debatable whether or not they
improve readability over shorter lines with flag definitions and comments
interleaved, or over removing the comments, since they are duplicated in
the other inode flag enumeration. I'd be happy to change it if desired.
The build time flag consistency check has been removed since it has already
shown the enums and flags to be consistent, and the flag definitions are very
difficult to get wrong now. It also means an extra list doesn't need to be
maintained. I can add it back if we're ultra paranoid.
I think the change to unsigned longs should be okay unless the flags are being
used perversely. I checked all uses and didn't see anything unusual. I also ran
the xfstests smoke test without issue (other than failure of generic/472, but
that is failing for me without this patch, too).
---
fs/ext4/ext4.h | 103 +++++++++++++++-----------------------------------------
fs/ext4/super.c | 3 --
2 files changed, 28 insertions(+), 78 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 33b3cac..6fd2698 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -36,6 +36,7 @@
#include <crypto/hash.h>
#include <linux/falloc.h>
#include <linux/percpu-rwsem.h>
+#include <linux/bitops.h>
#ifdef __KERNEL__
#include <linux/compat.h>
#endif
@@ -402,36 +403,33 @@ enum {
};
/*
- * Inode flags
- */
-#define EXT4_SECRM_FL 0x00000001 /* Secure deletion */
-#define EXT4_UNRM_FL 0x00000002 /* Undelete */
-#define EXT4_COMPR_FL 0x00000004 /* Compress file */
-#define EXT4_SYNC_FL 0x00000008 /* Synchronous updates */
-#define EXT4_IMMUTABLE_FL 0x00000010 /* Immutable file */
-#define EXT4_APPEND_FL 0x00000020 /* writes to file may only append */
-#define EXT4_NODUMP_FL 0x00000040 /* do not dump file */
-#define EXT4_NOATIME_FL 0x00000080 /* do not update atime */
-/* Reserved for compression usage... */
-#define EXT4_DIRTY_FL 0x00000100
-#define EXT4_COMPRBLK_FL 0x00000200 /* One or more compressed clusters */
-#define EXT4_NOCOMPR_FL 0x00000400 /* Don't compress */
- /* nb: was previously EXT2_ECOMPR_FL */
-#define EXT4_ENCRYPT_FL 0x00000800 /* encrypted file */
-/* End compression flags --- maybe not all used */
-#define EXT4_INDEX_FL 0x00001000 /* hash-indexed directory */
-#define EXT4_IMAGIC_FL 0x00002000 /* AFS directory */
-#define EXT4_JOURNAL_DATA_FL 0x00004000 /* file data should be journaled */
-#define EXT4_NOTAIL_FL 0x00008000 /* file tail should not be merged */
-#define EXT4_DIRSYNC_FL 0x00010000 /* dirsync behaviour (directories only) */
-#define EXT4_TOPDIR_FL 0x00020000 /* Top of directory hierarchies*/
-#define EXT4_HUGE_FILE_FL 0x00040000 /* Set to each huge file */
-#define EXT4_EXTENTS_FL 0x00080000 /* Inode uses extents */
-#define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
-#define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
-#define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
-#define EXT4_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
-#define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
+ * Inode flags used for bit masks
+ */
+#define EXT4_SECRM_FL BIT(EXT4_INODE_SECRM) /* Secure deletion */
+#define EXT4_UNRM_FL BIT(EXT4_INODE_UNRM) /* Undelete */
+#define EXT4_COMPR_FL BIT(EXT4_INODE_COMPR) /* Compress file */
+#define EXT4_SYNC_FL BIT(EXT4_INODE_SYNC) /* Synchronous updates */
+#define EXT4_IMMUTABLE_FL BIT(EXT4_INODE_IMMUTABLE) /* Immutable file */
+#define EXT4_APPEND_FL BIT(EXT4_INODE_APPEND) /* writes to file may only append */
+#define EXT4_NODUMP_FL BIT(EXT4_INODE_NODUMP) /* do not dump file */
+#define EXT4_NOATIME_FL BIT(EXT4_INODE_NOATIME) /* do not update atime */
+#define EXT4_DIRTY_FL BIT(EXT4_INODE_DIRTY)
+#define EXT4_COMPRBLK_FL BIT(EXT4_INODE_COMPRBLK) /* One or more compressed clusters */
+#define EXT4_NOCOMPR_FL BIT(EXT4_INODE_NOCOMPR) /* Don't compress */
+#define EXT4_ENCRYPT_FL BIT(EXT4_INODE_ENCRYPT) /* encrypted file, previously EXT2_ECOMPR_FL */
+#define EXT4_INDEX_FL BIT(EXT4_INODE_INDEX) /* hash-indexed directory */
+#define EXT4_IMAGIC_FL BIT(EXT4_INODE_IMAGIC) /* AFS directory */
+#define EXT4_JOURNAL_DATA_FL BIT(EXT4_INODE_JOURNAL_DATA) /* file data should be journaled */
+#define EXT4_NOTAIL_FL BIT(EXT4_INODE_NOTAIL) /* file tail should not be merged */
+#define EXT4_DIRSYNC_FL BIT(EXT4_INODE_DIRSYNC) /* dirsync behaviour (directories only) */
+#define EXT4_TOPDIR_FL BIT(EXT4_INODE_TOPDIR) /* Top of directory hierarchies*/
+#define EXT4_HUGE_FILE_FL BIT(EXT4_INODE_HUGE_FILE) /* Set to each huge file */
+#define EXT4_EXTENTS_FL BIT(EXT4_INODE_EXTENTS) /* Inode uses extents */
+#define EXT4_EA_INODE_FL BIT(EXT4_INODE_EA_INODE) /* Inode used for large EA */
+#define EXT4_EOFBLOCKS_FL BIT(EXT4_INODE_EOFBLOCKS) /* Blocks allocated beyond EOF */
+#define EXT4_INLINE_DATA_FL BIT(EXT4_INODE_INLINE_DATA) /* Inode has inline data. */
+#define EXT4_PROJINHERIT_FL BIT(EXT4_INODE_PROJINHERIT) /* Create with parents projid */
+#define EXT4_RESERVED_FL BIT(EXT4_INODE_RESERVED) /* reserved for ext4 lib */
#define EXT4_FL_USER_VISIBLE 0x304BDFFF /* User visible flags */
#define EXT4_FL_USER_MODIFIABLE 0x204BC0FF /* User modifiable flags */
@@ -468,51 +466,6 @@ static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
return flags & EXT4_OTHER_FLMASK;
}
-/*
- * Since it's pretty easy to mix up bit numbers and hex values, we use a
- * build-time check to make sure that EXT4_XXX_FL is consistent with respect to
- * EXT4_INODE_XXX. If all is well, the macros will be dropped, so, it won't cost
- * any extra space in the compiled kernel image, otherwise, the build will fail.
- * It's important that these values are the same, since we are using
- * EXT4_INODE_XXX to test for flag values, but EXT4_XXX_FL must be consistent
- * with the values of FS_XXX_FL defined in include/linux/fs.h and the on-disk
- * values found in ext2, ext3 and ext4 filesystems, and of course the values
- * defined in e2fsprogs.
- *
- * It's not paranoia if the Murphy's Law really *is* out to get you. :-)
- */
-#define TEST_FLAG_VALUE(FLAG) (EXT4_##FLAG##_FL == (1 << EXT4_INODE_##FLAG))
-#define CHECK_FLAG_VALUE(FLAG) BUILD_BUG_ON(!TEST_FLAG_VALUE(FLAG))
-
-static inline void ext4_check_flag_values(void)
-{
- CHECK_FLAG_VALUE(SECRM);
- CHECK_FLAG_VALUE(UNRM);
- CHECK_FLAG_VALUE(COMPR);
- CHECK_FLAG_VALUE(SYNC);
- CHECK_FLAG_VALUE(IMMUTABLE);
- CHECK_FLAG_VALUE(APPEND);
- CHECK_FLAG_VALUE(NODUMP);
- CHECK_FLAG_VALUE(NOATIME);
- CHECK_FLAG_VALUE(DIRTY);
- CHECK_FLAG_VALUE(COMPRBLK);
- CHECK_FLAG_VALUE(NOCOMPR);
- CHECK_FLAG_VALUE(ENCRYPT);
- CHECK_FLAG_VALUE(INDEX);
- CHECK_FLAG_VALUE(IMAGIC);
- CHECK_FLAG_VALUE(JOURNAL_DATA);
- CHECK_FLAG_VALUE(NOTAIL);
- CHECK_FLAG_VALUE(DIRSYNC);
- CHECK_FLAG_VALUE(TOPDIR);
- CHECK_FLAG_VALUE(HUGE_FILE);
- CHECK_FLAG_VALUE(EXTENTS);
- CHECK_FLAG_VALUE(EA_INODE);
- CHECK_FLAG_VALUE(EOFBLOCKS);
- CHECK_FLAG_VALUE(INLINE_DATA);
- CHECK_FLAG_VALUE(PROJINHERIT);
- CHECK_FLAG_VALUE(RESERVED);
-}
-
/* Used to pass group descriptor data when online resize is done */
struct ext4_new_group_input {
__u32 group; /* Group number for this data */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 7c46693..ec96765 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5790,9 +5790,6 @@ static int __init ext4_init_fs(void)
ext4_li_info = NULL;
mutex_init(&ext4_li_mtx);
- /* Build-time check for flags consistency */
- ext4_check_flag_values();
Signed-off-by: Tyson Nottingham <[email protected]>
---
This is just a code move to make the next patch diff cleaner.
---
fs/ext4/ext4.h | 66 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 4e091ea..33b3cac 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -369,6 +369,39 @@ struct flex_groups {
#define EXT4_N_BLOCKS (EXT4_TIND_BLOCK + 1)
/*
+ * Inode flags used for atomic set/get
+ */
+enum {
+ EXT4_INODE_SECRM = 0, /* Secure deletion */
+ EXT4_INODE_UNRM = 1, /* Undelete */
+ EXT4_INODE_COMPR = 2, /* Compress file */
+ EXT4_INODE_SYNC = 3, /* Synchronous updates */
+ EXT4_INODE_IMMUTABLE = 4, /* Immutable file */
+ EXT4_INODE_APPEND = 5, /* writes to file may only append */
+ EXT4_INODE_NODUMP = 6, /* do not dump file */
+ EXT4_INODE_NOATIME = 7, /* do not update atime */
+/* Reserved for compression usage... */
+ EXT4_INODE_DIRTY = 8,
+ EXT4_INODE_COMPRBLK = 9, /* One or more compressed clusters */
+ EXT4_INODE_NOCOMPR = 10, /* Don't compress */
+ EXT4_INODE_ENCRYPT = 11, /* Encrypted file */
+/* End compression flags --- maybe not all used */
+ EXT4_INODE_INDEX = 12, /* hash-indexed directory */
+ EXT4_INODE_IMAGIC = 13, /* AFS directory */
+ EXT4_INODE_JOURNAL_DATA = 14, /* file data should be journaled */
+ EXT4_INODE_NOTAIL = 15, /* file tail should not be merged */
+ EXT4_INODE_DIRSYNC = 16, /* dirsync behaviour (directories only) */
+ EXT4_INODE_TOPDIR = 17, /* Top of directory hierarchies*/
+ EXT4_INODE_HUGE_FILE = 18, /* Set to each huge file */
+ EXT4_INODE_EXTENTS = 19, /* Inode uses extents */
+ EXT4_INODE_EA_INODE = 21, /* Inode used for large EA */
+ EXT4_INODE_EOFBLOCKS = 22, /* Blocks allocated beyond EOF */
+ EXT4_INODE_INLINE_DATA = 28, /* Data in inode. */
+ EXT4_INODE_PROJINHERIT = 29, /* Create with parents projid */
+ EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
+};
+
+/*
* Inode flags
*/
#define EXT4_SECRM_FL 0x00000001 /* Secure deletion */
@@ -436,39 +469,6 @@ static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
}
/*
- * Inode flags used for atomic set/get
- */
-enum {
- EXT4_INODE_SECRM = 0, /* Secure deletion */
- EXT4_INODE_UNRM = 1, /* Undelete */
- EXT4_INODE_COMPR = 2, /* Compress file */
- EXT4_INODE_SYNC = 3, /* Synchronous updates */
- EXT4_INODE_IMMUTABLE = 4, /* Immutable file */
- EXT4_INODE_APPEND = 5, /* writes to file may only append */
- EXT4_INODE_NODUMP = 6, /* do not dump file */
- EXT4_INODE_NOATIME = 7, /* do not update atime */
-/* Reserved for compression usage... */
- EXT4_INODE_DIRTY = 8,
- EXT4_INODE_COMPRBLK = 9, /* One or more compressed clusters */
- EXT4_INODE_NOCOMPR = 10, /* Don't compress */
- EXT4_INODE_ENCRYPT = 11, /* Encrypted file */
-/* End compression flags --- maybe not all used */
- EXT4_INODE_INDEX = 12, /* hash-indexed directory */
- EXT4_INODE_IMAGIC = 13, /* AFS directory */
- EXT4_INODE_JOURNAL_DATA = 14, /* file data should be journaled */
- EXT4_INODE_NOTAIL = 15, /* file tail should not be merged */
- EXT4_INODE_DIRSYNC = 16, /* dirsync behaviour (directories only) */
- EXT4_INODE_TOPDIR = 17, /* Top of directory hierarchies*/
- EXT4_INODE_HUGE_FILE = 18, /* Set to each huge file */
- EXT4_INODE_EXTENTS = 19, /* Inode uses extents */
- EXT4_INODE_EA_INODE = 21, /* Inode used for large EA */
- EXT4_INODE_EOFBLOCKS = 22, /* Blocks allocated beyond EOF */
- EXT4_INODE_INLINE_DATA = 28, /* Data in inode. */
- EXT4_INODE_PROJINHERIT = 29, /* Create with parents projid */
- EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
-};
Define visible and modifiable inode flag masks in terms of inode flags
instead of hexadecimal to make it clearer which flags are included.
Signed-off-by: Tyson Nottingham <[email protected]>
---
I printed and diffed the flag values before and after to verify they match.
---
fs/ext4/ext4.h | 73 ++++++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 58 insertions(+), 15 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 6fd2698..9fe70ab 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -431,23 +431,66 @@ enum {
#define EXT4_PROJINHERIT_FL BIT(EXT4_INODE_PROJINHERIT) /* Create with parents projid */
#define EXT4_RESERVED_FL BIT(EXT4_INODE_RESERVED) /* reserved for ext4 lib */
-#define EXT4_FL_USER_VISIBLE 0x304BDFFF /* User visible flags */
-#define EXT4_FL_USER_MODIFIABLE 0x204BC0FF /* User modifiable flags */
-
-/* Flags we can manipulate with through EXT4_IOC_FSSETXATTR */
-#define EXT4_FL_XFLAG_VISIBLE (EXT4_SYNC_FL | \
- EXT4_IMMUTABLE_FL | \
- EXT4_APPEND_FL | \
- EXT4_NODUMP_FL | \
- EXT4_NOATIME_FL | \
- EXT4_PROJINHERIT_FL)
+/* Flags we can manipulate through EXT4_IOC_GETFLAGS */
+#define EXT4_FL_USER_VISIBLE (EXT4_SECRM_FL | \
+ EXT4_UNRM_FL | \
+ EXT4_COMPR_FL | \
+ EXT4_SYNC_FL | \
+ EXT4_IMMUTABLE_FL | \
+ EXT4_APPEND_FL | \
+ EXT4_NODUMP_FL | \
+ EXT4_NOATIME_FL | \
+ EXT4_DIRTY_FL | \
+ EXT4_COMPRBLK_FL | \
+ EXT4_NOCOMPR_FL | \
+ EXT4_ENCRYPT_FL | \
+ EXT4_INDEX_FL | \
+ EXT4_JOURNAL_DATA_FL | \
+ EXT4_NOTAIL_FL | \
+ EXT4_DIRSYNC_FL | \
+ EXT4_TOPDIR_FL | \
+ EXT4_EXTENTS_FL | \
+ EXT4_EOFBLOCKS_FL | \
+ EXT4_INLINE_DATA_FL | \
+ EXT4_PROJINHERIT_FL)
+
+/* Flags we can manipulate through EXT4_IOC_SETFLAGS */
+#define EXT4_FL_USER_MODIFIABLE (EXT4_SECRM_FL | \
+ EXT4_UNRM_FL | \
+ EXT4_COMPR_FL | \
+ EXT4_SYNC_FL | \
+ EXT4_IMMUTABLE_FL | \
+ EXT4_APPEND_FL | \
+ EXT4_NODUMP_FL | \
+ EXT4_NOATIME_FL | \
+ EXT4_JOURNAL_DATA_FL | \
+ EXT4_NOTAIL_FL | \
+ EXT4_DIRSYNC_FL | \
+ EXT4_TOPDIR_FL | \
+ EXT4_EXTENTS_FL | \
+ EXT4_EOFBLOCKS_FL | \
+ EXT4_PROJINHERIT_FL)
+
+/* Flags we can manipulate through EXT4_IOC_FSSETXATTR */
+#define EXT4_FL_XFLAG_VISIBLE (EXT4_SYNC_FL | \
+ EXT4_IMMUTABLE_FL | \
+ EXT4_APPEND_FL | \
+ EXT4_NODUMP_FL | \
+ EXT4_NOATIME_FL | \
+ EXT4_PROJINHERIT_FL)
/* Flags that should be inherited by new inodes from their parent. */
-#define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
- EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
- EXT4_NOCOMPR_FL | EXT4_JOURNAL_DATA_FL |\
- EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL |\
- EXT4_PROJINHERIT_FL)
+#define EXT4_FL_INHERITED (EXT4_SECRM_FL | \
+ EXT4_UNRM_FL | \
+ EXT4_COMPR_FL | \
+ EXT4_SYNC_FL | \
+ EXT4_NODUMP_FL | \
+ EXT4_NOATIME_FL | \
+ EXT4_NOCOMPR_FL | \
+ EXT4_JOURNAL_DATA_FL | \
+ EXT4_NOTAIL_FL | \
+ EXT4_DIRSYNC_FL | \
+ EXT4_PROJINHERIT_FL)
/* Flags that are appropriate for regular files (all but dir-specific ones). */
#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL))
--
2.7.4
On Fri 05-01-18 01:23:22, Tyson Nottingham wrote:
> This is my first patch to ext4 and the kernel. It's simple, but you
> might give it extra scrutiny since I'm new and all.
>
> I'm working off the dev branch of git://git.kernel.org/pub/scm/linux/
> kernel/git/tytso/ext4.git. Let me know if that's the wrong place to be.
I like the cleanup and the patches look good. You can add to all the
patches:
Reviewed-by: Jan Kara <[email protected]>
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR