Re: [PATCH 19/17] mkfs: increase default log size for new (aka bigtime) filesystems

[Eric Sandeen <esandeen@redhat.com>]

On 2/28/22 8:21 PM, Darrick J. Wong wrote:
> On Mon, Feb 28, 2022 at 03:44:29PM -0600, Eric Sandeen wrote:
>> On 2/25/22 8:54 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <djwong@kernel.org>
>>
>> ...
>>
>>> Hence we increase the ratio by 16x because there doesn't seem to be much
>>> improvement beyond that, and we don't want the log to grow /too/ large.
>>> This change does not affect filesystems larger than 4TB, nor does it
>>> affect formatting to older formats.
>>>
>>> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
>>> ---
>>>  mkfs/xfs_mkfs.c |   12 +++++++++++-
>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>>> index 96682f9a..7178d798 100644
>>> --- a/mkfs/xfs_mkfs.c
>>> +++ b/mkfs/xfs_mkfs.c
>>> @@ -3308,7 +3308,17 @@ _("external log device size %lld blocks too small, must be at least %lld blocks\
>>>  
>>>  	/* internal log - if no size specified, calculate automatically */
>>>  	if (!cfg->logblocks) {
>>> -		if (cfg->dblocks < GIGABYTES(1, cfg->blocklog)) {
>>> +		if (cfg->sb_feat.bigtime) {
>>
>> I'm not very keen on conditioning this on bigtime; it seems very ad-hoc and
>> unexpected. Future maintainers will look at this and wonder why bigtime is
>> in any way related to log size...
>>
>> If we make this change, why not just make it regardless of other features?
>> Is there some other risk to doing so?
> 
> I wrote it this way to leave the formatting behavior unchanged on older
> filesystems, figuring that you'd be wary of anything that could generate
> bug reports about old fs formats, e.g. "Why does my cloud deployment
> image generator report that the minified filesystem size went up when I
> went from X to X+1?"

We might run into that, but I'm perhaps naively willing to tell people that
if they were hard-coding reverse-engineered filesystem heuristics to the
nearest kilobyte, too bad so sad, they were doing it wrong.

> So now that I've guessed incorrectly, I guess I'll go change this to do
> it unconditionally.  Or drop the whole thing entirely.  I don't know.
> I'm too burned out to be able to think reasonably anymore.

I (maybe also naively) think it's reasonable to increase the log size for
small filesystems, given that they often as not become large filesystems
these days, and the ultimate % increase in size will be negligible.

(It's somewhere on my todo list to figure out how various products and
provisioning mechanisms actually create, transport, and expand these
minimized images, to see what the requirements really are ...)

> Frankly, I don't have the time to prove beyond a reasonable doubt that
> this the problem is exactly as stated, that the code change is exactly
> the correct fix, that no other fix is more appropriate, and that there
> are no other possible explanations for the slowness being complained
> aobut.  I really just wanted to do this one little thing that we've all
> basically agreed is the right thing.

I do agree. I just think that if the concern is a distro noticing the
difference, then the distro can carry the patch to get rid of that difference.
That's something I have had to do as well a times. With my upstream hat on, I
would rather keep distro version concerns out of the upstream patch, is all.

> Instead I'll just leave things as they are, and consider whether there
> even is a future for me working on XFS.

I'd miss you a lot if that happened. :(

-Eric

> --D
> 
>> Thanks,
>> -Eric