Re: [PATCH] ext4: fix overhead calculation in bigalloc filesystem (Re: ... )

[Zheng Liu <gnehzuil.liu@gmail.com>]

On Thu, Feb 21, 2013 at 01:52:58PM +0100, Lukáš Czerner wrote:
> On Thu, 21 Feb 2013, Lukáš Czerner wrote:
> 
> > Date: Thu, 21 Feb 2013 13:50:03 +0100 (CET)
> > From: Lukáš Czerner <lczerner@redhat.com>
> > To: Lukáš Czerner <lczerner@redhat.com>
> > Cc: Zheng Liu <gnehzuil.liu@gmail.com>, linux-ext4@vger.kernel.org,
> >     Theodore Ts'o <tytso@mit.edu>
> > Subject: Re: [PATCH] ext4: fix overhead calculation in bigalloc filesystem
> >     (Re: ... )
> > 
> > On Thu, 21 Feb 2013, Lukáš Czerner wrote:
> > 
> > ..snip...
> > 
> > > 
> > > Hi Zheng,
> > > 
> > > thanks for the review. I know about the other issues and I'm trying
> > > to resolve those as well. Right now I have a patch which includes
> > > the changes ext4_calculate_overhead() you've described below and more,
> > > but even with this I still see some problems remaining.
> > > 
> > > Hopefully will send another patch soon.
> > > 
> > > Thanks!
> > > -Lukas
> > > 
> > > > 
> > > > Subject: [PATCH] ext4: fix overhead calculation in bigalloc filesystem
> > > > 
> > > > From: Zheng Liu <wenqing.lz@taobao.com>
> > > > 
> > > > ext4_calculate_overhead() should compute the overhead and stash it in
> > > > sbi->s_overhead.  But we miss use EXT4_B2C() to calculate the number of
> > > > clusters before first_data_block and the number of journal blocks.  This
> > > > commit use EXT4_NUM_B2C() instead of EXT4_B2C() to calculate the
> > > > overhead.
> > > > 
> > > > Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
> > > > Cc: "Theodore Ts'o" <tytso@mit.edu>
> > > > ---
> > > >  fs/ext4/super.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > > > index 3d4fb81..6165558 100644
> > > > --- a/fs/ext4/super.c
> > > > +++ b/fs/ext4/super.c
> > > > @@ -3219,7 +3219,7 @@ int ext4_calculate_overhead(struct super_block *sb)
> > > >  	/*
> > > >  	 * All of the blocks before first_data_block are overhead
> > > >  	 */
> > > > -	overhead = EXT4_B2C(sbi, le32_to_cpu(es->s_first_data_block));
> > > > +	overhead = EXT4_NUM_B2C(sbi, le32_to_cpu(es->s_first_data_block));
> > 
> > ...except this. I do not think this is right because we do not skip
> > the first cluster right ? We're still using it, but we can never use
> > the block before es->s_first_data_block. Please correct me if I am
> > wrong.

Yes, I think you are right.

> 
> moreover we do not allow bigalloc file system with block size < 4k.

No, we allow user to use bigalloc with block size < 4k, such as:

  mkfs.ext4 -b 1024 -C 4096 -O bigalloc ${dev}

This command formats a bigalloc filesystem with blocksize = 1k and
clustersize = 4k, at least in e2fsprogs 1.42.7 it works well.

> 
> > 
> > 
> > > >  
> > > >  	/*
> > > >  	 * Add the overhead found in each block group
> > > > @@ -3235,7 +3235,7 @@ int ext4_calculate_overhead(struct super_block *sb)
> > > >  	}
> > > >  	/* Add the journal blocks as well */
> > > >  	if (sbi->s_journal)
> > > > -		overhead += EXT4_B2C(sbi, sbi->s_journal->j_maxlen);
> > > > +		overhead += EXT4_NUM_B2C(sbi, sbi->s_journal->j_maxlen);
> > 
> > This I already have in my patch I'm testing right now. And as I said
> > there are other places where we misuse EXT4_B2C().
> > 
> > -Lukas
> > 
> > > >  
> > > >  	sbi->s_overhead = overhead;
> > > >  	smp_wmb();
> > > > 
> > > 
> > 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html