Re: [PATCH 9/9] common/attr: Reduce MAX_ATTRS to leave some overhead for 64K blocksize

[Ritesh Harjani <riteshh@linux.ibm.com>]

On 21/06/30 03:20PM, Theodore Ts'o wrote:
> On Wed, Jun 30, 2021 at 08:51:50AM -0700, Darrick J. Wong wrote:
> > On Mon, Jun 14, 2021 at 11:58:13AM +0530, Ritesh Harjani wrote:
> > > Test generic/020 fails for ext4 with 64K blocksize. So increase some overhead
> > > value to reduce the MAX_ATTRS so that it can accomodate for 64K blocksize.
> > >
> > > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > > ---
> > >  common/attr | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/common/attr b/common/attr
> > > index d3902346..e8661d80 100644
> > > --- a/common/attr
> > > +++ b/common/attr
> > > @@ -260,7 +260,7 @@ xfs|udf|pvfs2|9p|ceph|nfs)
> > >  	# Assume max ~1 block of attrs
> > >  	BLOCK_SIZE=`_get_block_size $TEST_DIR`
> > >  	# user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
> > > -	let MAX_ATTRS=$BLOCK_SIZE/40
> > > +	let MAX_ATTRS=$BLOCK_SIZE/48
> >
> > 50% is quite a lot of overhead; maybe we should special-case this?
>
> The problem is that 32 bytes is an underestimate when i > 99 for
> user.attribute_$i=value_$i.  And with a 4k blocksize, MAX_ATTRS =
> 4096 / 40 = 102.
>
> The exact calculation for ext4 is:
>
> fixed_block_overhead = 32
> fixed_entry_overhead = 16
> max_attr = (block_size - fixed_block_overhead) /
> 	(fixed_entry_overhead + round_up(len(attr_name), 4) +
> 	 round_up(len(value), 4))
>
> For 4k blocksizes, most of the attributes have an attr_name of
> "attribute_NN" which is 8, and "value_NN" which is 12.
>
> But for larger block sizes, we start having extended attributes of the
> form "attribute_NNN" or "attribute_NNNN", and "value_NNN" and
> "value_NNNN", which causes the round(len(..), 4) to jump up by 4
> bytes.  So round_up(len(attr_name, 4)) becomes 12 instead of 8, and
> round_up(len(value, 4)) becomes 16 instead of 12.  So:
>
> 	max_attrs = (block_size - 32) / (16 + 12 + 16)
> or
> 	max_attrs = (block_size - 32) / 44
>
> instead of:
>
> 	max_attrs = (block_size - 32) / (16 + 8 + 12)
> or
> 	max_attrs = (block_size - 32) / 36

Thanks for the indepth details. Yes, in my testing as well it was coming to be
around ~44%. But to be safe I chose 50%.
I verified from the code as well. We do have 32 bytes of overhead per block for
the the header. And per entry overhead of 16 bytes. The rounding happens for
both name (EXT4_XATTR_LEN) and value (EXT4_XATTR_SIZE) of attr.

Perhaps, it will be helpful if we update above info in ext4 documentation as
well. In that the rounding off is only mentioned for value and not for name
length.

>
> So special casing things for block sizes > 4k may very well make
> sense.  Perhaps it's even worth it to put in an ext[234] specific,
> exalc calculation for MAX_ATTRS in common/attr.

yes, will make these changes for ext[234] specific in common/attr.

-ritesh