Re: [PATCH] ext4: Improve feature checking
On 6/30/24 20:23, Sean Anderson wrote:
On 6/30/24 17:25, Richard Weinberger wrote:
Evaluate the filesystem incompat and ro_compat bit fields to judge
whether the filesystem can be read or written.
For the read side only a scary warning is shown so far.
I'd love to about mounting too, but I fear this will break some setups
I think you are missing a verb in the first half of your sentence
where the driver works by chance.
Signed-off-by: Richard Weinberger
---
After facing ext4 write corruptions and read errors I checked
the ext4 implementation briefly.
Hopefully this patch helps other users to detect earlier that
they're using ext4 features which are not supported by u-boot.
To make feature checking possible I had to figure what this
implementation actually supports.
I hope I got them right, feedback is welcome!
Thanks,
//richard
---
fs/ext4/ext4_common.c | 8 +++
fs/ext4/ext4_write.c | 12 --
include/ext4fs.h | 55 ++-
3 files changed, 72 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 2ff0dca249..7148d35ee0 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2386,6 +2386,14 @@ int ext4fs_mount(void)
fs->inodesz = 128;
fs->gdsize = 32;
} else {
+ int missing = __le32_to_cpu(data->sblock.feature_incompat) & \
+ ~(EXT4_FEATURE_INCOMPAT_SUPP | \
+ EXT4_FEATURE_INCOMPAT_SUPP_LAZY_RO);
+
+ if (missing)
+ log_err("fs uses incompatible features: %08x, failures *are*
expected!\n",
+ missing);
+
I think it is a bit unclear to the user what their action should be. Maybe
something like
printf("EXT4 incompatible features: %08x, ignoring...\n")
and then maybe add a comment like what you have in the commit message.
or maybe even
printf("EXT4 incompatible features: %08x, mounting read-only...\n")
debug("EXT4 features COMPAT: %08x INCOMPAT: %08x RO_COMPAT: %08x\n",
__le32_to_cpu(data->sblock.feature_compatibility),
__le32_to_cpu(data->sblock.feature_incompat),
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index d057f6b5a7..4aae3c5f7f 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -869,6 +869,7 @@ int ext4fs_write(const char *fname, const char *buffer,
ALLOC_CACHE_ALIGN_BUFFER(char, filename, 256);
bool store_link_in_inode = false;
memset(filename, 0x00, 256);
+ int missing_feat;
if (type != FILETYPE_REG && type != FILETYPE_SYMLINK)
return -1;
@@ -882,8 +883,15 @@ int ext4fs_write(const char *fname, const char *buffer,
return -1;
}
- if (le32_to_cpu(fs->sb->feature_ro_compat) &
EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) {
- printf("Unsupported feature metadata_csum found, not writing.\n");
+ missing_feat = le32_to_cpu(fs->sb->feature_incompat) &
~EXT4_FEATURE_INCOMPAT_SUPP;
+ if (missing_feat) {
+ log_err("Unsupported features found %08x, not writing.\n",
missing_feat);
+ return -1;
+ }
+
+ missing_feat = le32_to_cpu(fs->sb->feature_ro_compat) &
~EXT4_FEATURE_RO_COMPAT_SUPP;
+ if (missing_feat) {
+ log_err("Unsupported RO compat features found %08x, not writing.\n",
missing_feat);
return -1;
}
diff --git a/include/ext4fs.h b/include/ext4fs.h
index d96edfd057..01b66f469f 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -34,12 +34,65 @@ struct disk_partition;
#define EXT4_TOPDIR_FL 0x0002 /* Top of directory hierarchies*/
#define EXT4_EXTENTS_FL 0x0008 /* Inode uses extents */
#define EXT4_EXT_MAGIC 0xf30a
-#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM 0x0010
+
+#define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER 0x0001
+#define EXT4_FEATURE_RO_COMPAT_LARGE_FILE 0x0002
+#define EXT4_FEATURE_RO_COMPAT_BTREE_DIR 0x0004
+#define EXT4_FEATURE_RO_COMPAT_HUGE_FILE 0x0008
+#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM 0x0010
+#define EXT4_FEATURE_RO_COMPAT_DIR_NLINK 0x0020
+#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE 0x0040
+#define EXT4_FEATURE_RO_COMPAT_QUOTA 0x0100
+#define EXT4_FEATURE_RO_COMPAT_BIGALLOC 0x0200
#define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM 0x0400
+
+#define EXT4_FEATURE_INCOMPAT_FILETYPE 0x0002
+#define EXTE_FEATURE_INCOMPAT_RECOVER 0x0004
#define EXT4_FEATURE_INCOMPAT_EXTENTS 0x0040
#define EXT4_FEATURE_INCOMPAT_64BIT 0x0080
+#define EXT4_FEATURE_INCOMPAT_MMP 0x0100
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG 0x0200
+#define EXT4_FEATURE_INCOMPAT_CSUM_SEED 0x2000
+#define EXT4_FEATURE_INCOMPAT_ENCRYPT 0x1
+
#define EXT4_INDIRECT_BLOCKS 12
+/*
+ * Incompat features supported by this implementation.
+ */
+#define EXT4_FEATURE_INCOMPAT_SUPP ( EXT4_FEATURE_INCOMPAT_FILETYPE \
+ | EXTE_FEATURE_INCOMPAT_RECOVER \
EXT4
+ | EXT4_FEATURE_INCOMPAT_EXTENTS \
+
Re: [PATCH] ext4: Improve feature checking
On 6/30/24 17:25, Richard Weinberger wrote:
Evaluate the filesystem incompat and ro_compat bit fields to judge
whether the filesystem can be read or written.
For the read side only a scary warning is shown so far.
I'd love to about mounting too, but I fear this will break some setups
I think you are missing a verb in the first half of your sentence
where the driver works by chance.
Signed-off-by: Richard Weinberger
---
After facing ext4 write corruptions and read errors I checked
the ext4 implementation briefly.
Hopefully this patch helps other users to detect earlier that
they're using ext4 features which are not supported by u-boot.
To make feature checking possible I had to figure what this
implementation actually supports.
I hope I got them right, feedback is welcome!
Thanks,
//richard
---
fs/ext4/ext4_common.c | 8 +++
fs/ext4/ext4_write.c | 12 --
include/ext4fs.h | 55 ++-
3 files changed, 72 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 2ff0dca249..7148d35ee0 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2386,6 +2386,14 @@ int ext4fs_mount(void)
fs->inodesz = 128;
fs->gdsize = 32;
} else {
+ int missing = __le32_to_cpu(data->sblock.feature_incompat) & \
+ ~(EXT4_FEATURE_INCOMPAT_SUPP | \
+ EXT4_FEATURE_INCOMPAT_SUPP_LAZY_RO);
+
+ if (missing)
+ log_err("fs uses incompatible features: %08x, failures *are*
expected!\n",
+ missing);
+
I think it is a bit unclear to the user what their action should be. Maybe
something like
printf("EXT4 incompatible features: %08x, ignoring...\n")
and then maybe add a comment like what you have in the commit message.
debug("EXT4 features COMPAT: %08x INCOMPAT: %08x RO_COMPAT:
%08x\n",
__le32_to_cpu(data->sblock.feature_compatibility),
__le32_to_cpu(data->sblock.feature_incompat),
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index d057f6b5a7..4aae3c5f7f 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -869,6 +869,7 @@ int ext4fs_write(const char *fname, const char *buffer,
ALLOC_CACHE_ALIGN_BUFFER(char, filename, 256);
bool store_link_in_inode = false;
memset(filename, 0x00, 256);
+ int missing_feat;
if (type != FILETYPE_REG && type != FILETYPE_SYMLINK)
return -1;
@@ -882,8 +883,15 @@ int ext4fs_write(const char *fname, const char *buffer,
return -1;
}
- if (le32_to_cpu(fs->sb->feature_ro_compat) & EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) {
- printf("Unsupported feature metadata_csum found, not
writing.\n");
+ missing_feat = le32_to_cpu(fs->sb->feature_incompat) &
~EXT4_FEATURE_INCOMPAT_SUPP;
+ if (missing_feat) {
+ log_err("Unsupported features found %08x, not writing.\n",
missing_feat);
+ return -1;
+ }
+
+ missing_feat = le32_to_cpu(fs->sb->feature_ro_compat) &
~EXT4_FEATURE_RO_COMPAT_SUPP;
+ if (missing_feat) {
+ log_err("Unsupported RO compat features found %08x, not
writing.\n", missing_feat);
return -1;
}
diff --git a/include/ext4fs.h b/include/ext4fs.h
index d96edfd057..01b66f469f 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -34,12 +34,65 @@ struct disk_partition;
#define EXT4_TOPDIR_FL0x0002 /* Top of directory
hierarchies*/
#define EXT4_EXTENTS_FL 0x0008 /* Inode uses extents */
#define EXT4_EXT_MAGIC0xf30a
-#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM0x0010
+
+#define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER 0x0001
+#define EXT4_FEATURE_RO_COMPAT_LARGE_FILE0x0002
+#define EXT4_FEATURE_RO_COMPAT_BTREE_DIR 0x0004
+#define EXT4_FEATURE_RO_COMPAT_HUGE_FILE 0x0008
+#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM 0x0010
+#define EXT4_FEATURE_RO_COMPAT_DIR_NLINK 0x0020
+#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE 0x0040
+#define EXT4_FEATURE_RO_COMPAT_QUOTA 0x0100
+#define EXT4_FEATURE_RO_COMPAT_BIGALLOC 0x0200
#define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM 0x0400
+
+#define EXT4_FEATURE_INCOMPAT_FILETYPE 0x0002
+#define EXTE_FEATURE_INCOMPAT_RECOVER 0x0004
#define EXT4_FEATURE_INCOMPAT_EXTENTS 0x0040
#define EXT4_FEATURE_INCOMPAT_64BIT 0x0080
+#define EXT4_FEATURE_INCOMPAT_MMP 0x0100
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG 0x0200
+#define EXT4_FEATURE_INCOMPAT_CSUM_SEED 0x2000
+#define EXT4_FEATURE_INCOMPAT_ENCRYPT 0x1
+
#define EXT4_INDIRECT_BLOCKS 12
+/*
+ * Incompat features supported by this implementation.
+ */
+#define EXT4_FEATURE_INCOMPAT_SUPP ( EXT4_FEATURE_INCOMPAT_FILETYPE \
+
[PATCH] ext4: Improve feature checking
Evaluate the filesystem incompat and ro_compat bit fields to judge
whether the filesystem can be read or written.
For the read side only a scary warning is shown so far.
I'd love to about mounting too, but I fear this will break some setups
where the driver works by chance.
Signed-off-by: Richard Weinberger
---
After facing ext4 write corruptions and read errors I checked
the ext4 implementation briefly.
Hopefully this patch helps other users to detect earlier that
they're using ext4 features which are not supported by u-boot.
To make feature checking possible I had to figure what this
implementation actually supports.
I hope I got them right, feedback is welcome!
Thanks,
//richard
---
fs/ext4/ext4_common.c | 8 +++
fs/ext4/ext4_write.c | 12 --
include/ext4fs.h | 55 ++-
3 files changed, 72 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 2ff0dca249..7148d35ee0 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2386,6 +2386,14 @@ int ext4fs_mount(void)
fs->inodesz = 128;
fs->gdsize = 32;
} else {
+ int missing = __le32_to_cpu(data->sblock.feature_incompat) & \
+ ~(EXT4_FEATURE_INCOMPAT_SUPP | \
+ EXT4_FEATURE_INCOMPAT_SUPP_LAZY_RO);
+
+ if (missing)
+ log_err("fs uses incompatible features: %08x, failures
*are* expected!\n",
+ missing);
+
debug("EXT4 features COMPAT: %08x INCOMPAT: %08x RO_COMPAT:
%08x\n",
__le32_to_cpu(data->sblock.feature_compatibility),
__le32_to_cpu(data->sblock.feature_incompat),
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index d057f6b5a7..4aae3c5f7f 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -869,6 +869,7 @@ int ext4fs_write(const char *fname, const char *buffer,
ALLOC_CACHE_ALIGN_BUFFER(char, filename, 256);
bool store_link_in_inode = false;
memset(filename, 0x00, 256);
+ int missing_feat;
if (type != FILETYPE_REG && type != FILETYPE_SYMLINK)
return -1;
@@ -882,8 +883,15 @@ int ext4fs_write(const char *fname, const char *buffer,
return -1;
}
- if (le32_to_cpu(fs->sb->feature_ro_compat) &
EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) {
- printf("Unsupported feature metadata_csum found, not
writing.\n");
+ missing_feat = le32_to_cpu(fs->sb->feature_incompat) &
~EXT4_FEATURE_INCOMPAT_SUPP;
+ if (missing_feat) {
+ log_err("Unsupported features found %08x, not writing.\n",
missing_feat);
+ return -1;
+ }
+
+ missing_feat = le32_to_cpu(fs->sb->feature_ro_compat) &
~EXT4_FEATURE_RO_COMPAT_SUPP;
+ if (missing_feat) {
+ log_err("Unsupported RO compat features found %08x, not
writing.\n", missing_feat);
return -1;
}
diff --git a/include/ext4fs.h b/include/ext4fs.h
index d96edfd057..01b66f469f 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -34,12 +34,65 @@ struct disk_partition;
#define EXT4_TOPDIR_FL 0x0002 /* Top of directory hierarchies*/
#define EXT4_EXTENTS_FL0x0008 /* Inode uses extents */
#define EXT4_EXT_MAGIC 0xf30a
-#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM0x0010
+
+#define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER 0x0001
+#define EXT4_FEATURE_RO_COMPAT_LARGE_FILE0x0002
+#define EXT4_FEATURE_RO_COMPAT_BTREE_DIR 0x0004
+#define EXT4_FEATURE_RO_COMPAT_HUGE_FILE 0x0008
+#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM 0x0010
+#define EXT4_FEATURE_RO_COMPAT_DIR_NLINK 0x0020
+#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE 0x0040
+#define EXT4_FEATURE_RO_COMPAT_QUOTA 0x0100
+#define EXT4_FEATURE_RO_COMPAT_BIGALLOC 0x0200
#define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM 0x0400
+
+#define EXT4_FEATURE_INCOMPAT_FILETYPE 0x0002
+#define EXTE_FEATURE_INCOMPAT_RECOVER 0x0004
#define EXT4_FEATURE_INCOMPAT_EXTENTS 0x0040
#define EXT4_FEATURE_INCOMPAT_64BIT0x0080
+#define EXT4_FEATURE_INCOMPAT_MMP 0x0100
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG 0x0200
+#define EXT4_FEATURE_INCOMPAT_CSUM_SEED 0x2000
+#define EXT4_FEATURE_INCOMPAT_ENCRYPT 0x1
+
#define EXT4_INDIRECT_BLOCKS 12
+/*
+ * Incompat features supported by this implementation.
+ */
+#define EXT4_FEATURE_INCOMPAT_SUPP ( EXT4_FEATURE_INCOMPAT_FILETYPE \
+ | EXTE_FEATURE_INCOMPAT_RECOVER \
+ | EXT4_FEATURE_INCOMPAT_EXTENTS \
+ | EXT4_FEATURE_INCOMPAT_64BIT \
+ | EXT4_FEATURE_INCOMPAT_FLEX_BG \
+ )
+
+/*
+ * Incompat features supported by this implementation only in a lazy
+ *
