* [PATCH] Fix EXT4_FEATURE_RO_COMPAT_HUGE_FILE check
@ 2010-08-24 1:34 Justin Maggard
2010-08-24 4:34 ` Andreas Dilger
2010-11-22 22:34 ` Ted Ts'o
0 siblings, 2 replies; 5+ messages in thread
From: Justin Maggard @ 2010-08-24 1:34 UTC (permalink / raw)
To: linux-ext4
Creating a 4TB file on a filesystem with the 64bit flag set results in e2fsck consistently complaining about i_blocks being wrong, with confusing messages like this:
Inode 29818882, i_blocks is 8388608816, should be 8388608816. Fix? no
That appears to be caused by ext2fs_inode_i_blocks() checking for the EXT4_FEATURE_RO_COMPAT_HUGE_FILE in the wrong place. Fix it.
Signed-off-by: Justin Maggard <jmaggard10@gmail.com>
---
lib/ext2fs/blknum.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
index a48b696..560edc7 100644
--- a/lib/ext2fs/blknum.c
+++ b/lib/ext2fs/blknum.c
@@ -49,7 +49,7 @@ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
struct ext2_inode *inode)
{
return (inode->i_blocks |
- ((fs->super->s_feature_incompat &
+ ((fs->super->s_feature_ro_compat &
EXT4_FEATURE_RO_COMPAT_HUGE_FILE) ?
(__u64) inode->osd2.linux2.l_i_blocks_hi << 32 : 0)) -
(inode->i_file_acl ? fs->blocksize >> 9 : 0);
@@ -62,7 +62,7 @@ blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,
struct ext2_inode *inode)
{
return (inode->i_blocks |
- ((fs->super->s_feature_incompat &
+ ((fs->super->s_feature_ro_compat &
EXT4_FEATURE_RO_COMPAT_HUGE_FILE) ?
(__u64)inode->osd2.linux2.l_i_blocks_hi << 32 : 0));
}
--
1.5.6.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix EXT4_FEATURE_RO_COMPAT_HUGE_FILE check
2010-08-24 1:34 [PATCH] Fix EXT4_FEATURE_RO_COMPAT_HUGE_FILE check Justin Maggard
@ 2010-08-24 4:34 ` Andreas Dilger
2010-08-24 16:41 ` Eric Sandeen
2010-11-22 22:34 ` Ted Ts'o
1 sibling, 1 reply; 5+ messages in thread
From: Andreas Dilger @ 2010-08-24 4:34 UTC (permalink / raw)
To: Justin Maggard; +Cc: linux-ext4
On 2010-08-23, at 19:34, Justin Maggard wrote:
> Creating a 4TB file on a filesystem with the 64bit flag set results in e2fsck consistently complaining about i_blocks being wrong, with confusing messages like this:
>
> That appears to be caused by ext2fs_inode_i_blocks() checking for the EXT4_FEATURE_RO_COMPAT_HUGE_FILE in the wrong place. Fix it.
>
> - ((fs->super->s_feature_incompat &
> + ((fs->super->s_feature_ro_compat &
> EXT4_FEATURE_RO_COMPAT_HUGE_FILE) ?
This isn't the first time that we've had problems like this, and it won't be the last time unless there is a better API for checking these flags.
I've long thought about having something like:
#define EXT4_CHECK_COMPAT(sb, flag) ((super)->s_feature_compat & \
EXT4_FEATURE_COMPAT_ ## flag)
#define EXT4_CHECK_RO_COMPAT(sb, flag) ((super)->s_feature_ro_compat & \
EXT4_FEATURE_RO_COMPAT_ ## flag)
#define EXT4_CHECK_INCOMPAT(sb, flag) ((super)->s_feature_incompat & \
EXT4_FEATURE_INCOMPAT_ ## flag)
#define EXT4_SET_COMPAT(sb, flag) do { (super)->s_feature_compat |= \
EXT4_FEATURE_COMPAT_ ## flag; } while(0)
#define EXT4_SET_RO_COMPAT(sb, flag) do { (super)->s_feature_ro_compat |= \
EXT4_FEATURE_RO_COMPAT_ ##flag;} while(0)
#define EXT4_SET_INCOMPAT(sb, flag) do { (super)->s_feature_incompat |= \
EXT4_FEATURE_INCOMPAT_ ## flag;} while(0)
This will produce a compile error if the flag name and the macro name don't match. Unfortunately, for e2fsprogs we will need 3 sets of such macros because the feature flags are named EXT2_*, EXT3_*, and EXT4_*, depending on when they were added. For the kernel code that wouldn't be needed.
Cheers, Andreas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix EXT4_FEATURE_RO_COMPAT_HUGE_FILE check
2010-08-24 4:34 ` Andreas Dilger
@ 2010-08-24 16:41 ` Eric Sandeen
2010-08-24 19:32 ` Justin Maggard
0 siblings, 1 reply; 5+ messages in thread
From: Eric Sandeen @ 2010-08-24 16:41 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Justin Maggard, linux-ext4
Andreas Dilger wrote:
> This isn't the first time that we've had problems like this, and it
> won't be the last time unless there is a better API for checking
> these flags.
*nod*
> I've long thought about having something like:
Same thing crossed my mind; seems like a very good idea.
> #define EXT4_CHECK_COMPAT(sb, flag) ((super)->s_feature_compat & \
> EXT4_FEATURE_COMPAT_ ## flag)
> #define EXT4_CHECK_RO_COMPAT(sb, flag) ((super)->s_feature_ro_compat & \
> EXT4_FEATURE_RO_COMPAT_ ## flag)
> #define EXT4_CHECK_INCOMPAT(sb, flag) ((super)->s_feature_incompat & \
> EXT4_FEATURE_INCOMPAT_ ## flag)
> #define EXT4_SET_COMPAT(sb, flag) do { (super)->s_feature_compat |= \
> EXT4_FEATURE_COMPAT_ ## flag; } while(0)
> #define EXT4_SET_RO_COMPAT(sb, flag) do { (super)->s_feature_ro_compat |= \
> EXT4_FEATURE_RO_COMPAT_ ##flag;} while(0)
> #define EXT4_SET_INCOMPAT(sb, flag) do { (super)->s_feature_incompat |= \
> EXT4_FEATURE_INCOMPAT_ ## flag;} while(0)
>
> This will produce a compile error if the flag name and the macro name
> don't match. Unfortunately, for e2fsprogs we will need 3 sets of such
> macros because the feature flags are named EXT2_*, EXT3_*, and
> EXT4_*, depending on when they were added. For the kernel code that
> wouldn't be needed.
Time to name them consistently in e2fsprogs, then, I think?
-Eric
> Cheers, Andreas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix EXT4_FEATURE_RO_COMPAT_HUGE_FILE check
2010-08-24 16:41 ` Eric Sandeen
@ 2010-08-24 19:32 ` Justin Maggard
0 siblings, 0 replies; 5+ messages in thread
From: Justin Maggard @ 2010-08-24 19:32 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Andreas Dilger, linux-ext4
On Tue, Aug 24, 2010 at 9:41 AM, Eric Sandeen <sandeen@redhat.com> wrote:
> Andreas Dilger wrote:
>
>> This isn't the first time that we've had problems like this, and it
>> won't be the last time unless there is a better API for checking
>> these flags.
>
> *nod*
>
On a related note (to my original problem)... is the bad i_blocks
count fixup code in the check_blocks() function of pass1.c broken for
large files, or am I misunderstanding the code? pb.num_blocks and
ext2fs_inode_i_blocks() are both >32 bits, but the fixup code appears
to always try to shove pb.num_blocks into the 32-bit inode->i_blocks
and set inode->osd2.linux2.l_i_blocks_hi = 0.
-Justin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix EXT4_FEATURE_RO_COMPAT_HUGE_FILE check
2010-08-24 1:34 [PATCH] Fix EXT4_FEATURE_RO_COMPAT_HUGE_FILE check Justin Maggard
2010-08-24 4:34 ` Andreas Dilger
@ 2010-11-22 22:34 ` Ted Ts'o
1 sibling, 0 replies; 5+ messages in thread
From: Ted Ts'o @ 2010-11-22 22:34 UTC (permalink / raw)
To: Justin Maggard; +Cc: linux-ext4
On Tue, Aug 24, 2010 at 01:34:16AM +0000, Justin Maggard wrote:
> Creating a 4TB file on a filesystem with the 64bit flag set results
> in e2fsck consistently complaining about i_blocks being wrong, with
> confusing messages like this:
>
> Inode 29818882, i_blocks is 8388608816, should be 8388608816. Fix? no
>
> That appears to be caused by ext2fs_inode_i_blocks() checking for
> the EXT4_FEATURE_RO_COMPAT_HUGE_FILE in the wrong place. Fix it.
>
> Signed-off-by: Justin Maggard <jmaggard10@gmail.com>
Applied to the e2fsprogs master branch. Sorry for the delay in
attending to this patch. This Fall has been incredibly busy for me;
now that I have the maint branch tidied up for the 1.41.12 release,
I'm not catching up with the backlog on the master branch.
- Ted
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-11-23 15:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-24 1:34 [PATCH] Fix EXT4_FEATURE_RO_COMPAT_HUGE_FILE check Justin Maggard
2010-08-24 4:34 ` Andreas Dilger
2010-08-24 16:41 ` Eric Sandeen
2010-08-24 19:32 ` Justin Maggard
2010-11-22 22:34 ` Ted Ts'o
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.