Re: [PATCH v2 2/2] common/rc: improve block size support for bcachefs

[Su Yue <l@damenly.org>]

On Wed 17 Jan 2024 at 12:55, Brian Foster <bfoster@redhat.com> 
wrote:

> On Wed, Jan 17, 2024 at 05:23:09PM +0800, Su Yue wrote:
>> For bcachefs, def_blksz is never assigned even MKFS_OPTIONS 
>> contains option
>> '--block_size'. So block size of bcachefs on scratch dev is 
>> always 4096
>> if _scratch_mkfs_sized is called without second parameter.
>>
>> Add the pattern to set def_blksz if '--block_size' is given in 
>> MKFS_OPTIONS.
>>
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> changelog:
>> v2:
>>     Born.
>> ---
>>  common/rc | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index 31c21d2a8360..6a01de69cf05 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -950,6 +950,9 @@ _scratch_mkfs_sized()
>>  	jfs)
>>  		def_blksz=4096
>>  		;;
>> +	bcachefs)
>> +		def_blksz=`echo $MKFS_OPTIONS | sed -rn 
>> 's/.*(--block_size)[ =]?+([0-9]+).*/\2/p'`
>> +		;;
>
> So if the default bcachefs block size is 512b, I wonder if this 
> should
> do something like what the udf branch does a few lines above and
>
mkfs.bcachefs decides block size by querying statbuf.st_blksize or
BLKPBSZGET from the device if the option is not given.

> override the hardcoded default of 4k. ISTM this whole thing 
> would be
> more robust if it just elided the param in the default cases and 
> let the
> associated mkfs tool use its own default, but that's probably a 
> separate
> issue. Hm?
>
Since there is no default block size of bcachefs, maybe we can let
mkfs.bcachefs decide on its own but it will make chaos when 
somebody
reports an unreproducible BUG due to the different block_size even
we have same local.config. It just happened...
So for now, I think 4096 is a resonable value of bcachefs block 
size.

--
Su
>
> Brian
>
>>  	esac
>>
>>  	[ -n "$def_blksz" ] && blocksize=$def_blksz
>> --
>> 2.43.0
>>