Re: [PATCH V1.1] xfs: Use struct xfs_bmdr_block instead of struct xfs_btree_block to calculate root node size

[Chandan Babu R <chandanrlinux@gmail.com>]

On 02 Apr 2021 at 21:09, Darrick J. Wong wrote:
> On Fri, Apr 02, 2021 at 05:21:00PM +0530, Chandan Babu R wrote:
>> The incore data fork of an inode stores the bmap btree root node as 'struct
>> xfs_btree_block'. However, the ondisk version of the inode stores the bmap
>> btree root node as a 'struct xfs_bmdr_block'.
>> 
>> xfs_bmap_add_attrfork_btree() checks if the btree root node fits inside the
>> data fork of the inode. However, it incorrectly uses 'struct xfs_btree_block'
>> to compute the size of the bmap btree root node. Since size of 'struct
>> xfs_btree_block' is larger than that of 'struct xfs_bmdr_block',
>> xfs_bmap_add_attrfork_btree() could end up unnecessarily demoting the current
>> root node as the child of newly allocated root node.
>> 
>> This commit optimizes space usage by modifying xfs_bmap_add_attrfork_btree()
>> to use 'struct xfs_bmdr_block' to check if the bmap btree root node fits
>> inside the data fork of the inode.
>
> Hmm.  This introduces a (compatible) change in the ondisk format, since
> we no longer promote the data fork btree root block unnecessarily, right?

Yes, that is correct.

>
> We've been writing out filesystems in that state for years, so I think
> scrub is going to need patching to disable the "could the root block
> contents fit in the inode root?" check on the data fork if there's an
> attr fork.

You are right. I will post the corresponding patch soon.

>
> Meanwhile, this fix looks decent.
>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>
> By the way, have you tried running xfs/{529-538} on a realtime
> filesystem formatted with -d rtinherit=1 ?  There's something odd
> causing them to fail, but it's realtime so who knows what that's
> about. :)

Thanks for reporting the bug. I will see what is going on there.

-- 
chandan