Re: [PATCH v5 04/18] btrfs: add simple_quota incompat feature to sysfs

[Boris Burkov <boris@bur.io>]

On Thu, Sep 07, 2023 at 01:28:25PM +0200, David Sterba wrote:
> On Thu, Jul 27, 2023 at 03:12:51PM -0700, Boris Burkov wrote:
> > Add an entry in the features directory for the new incompat flag
> > 
> > Signed-off-by: Boris Burkov <boris@bur.io>
> > ---
> >  fs/btrfs/sysfs.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> > index e53614753391..f62bba0068ca 100644
> > --- a/fs/btrfs/sysfs.c
> > +++ b/fs/btrfs/sysfs.c
> > @@ -291,6 +291,7 @@ BTRFS_FEAT_ATTR_INCOMPAT(metadata_uuid, METADATA_UUID);
> >  BTRFS_FEAT_ATTR_COMPAT_RO(free_space_tree, FREE_SPACE_TREE);
> >  BTRFS_FEAT_ATTR_COMPAT_RO(block_group_tree, BLOCK_GROUP_TREE);
> >  BTRFS_FEAT_ATTR_INCOMPAT(raid1c34, RAID1C34);
> > +BTRFS_FEAT_ATTR_INCOMPAT(simple_quota, SIMPLE_QUOTA);
> 
> I'm not sure if you mentioned in the cover letter or if we had discussed
> it before, but does this need to be a full incompat bit? I.e. no mount
> on older kernels, compared to a COMPAT_RO which would allow
> read-only mount.

Unfortunately, as it is, simple quotas does need a full incompat bit.
That is because of the details of how the kernel parses inline refs, but
essentially that code relies on item size being fully exhausted by an
iteration that steps forward by hard-code-computed inline ref size
chunks. That parsing code blows up on the new structures in a way that
can't be fixed, as far as I can tell.

To be COMPAT_RO, we would need to introduce an entirely new item. We
discussed this in one of the early discussions and concluded that was
not worth the space cost compared to an inline item.

> 
> >  #ifdef CONFIG_BLK_DEV_ZONED
> >  BTRFS_FEAT_ATTR_INCOMPAT(zoned, ZONED);
> >  #endif
> > @@ -322,6 +323,7 @@ static struct attribute *btrfs_supported_feature_attrs[] = {
> >  	BTRFS_FEAT_ATTR_PTR(free_space_tree),
> >  	BTRFS_FEAT_ATTR_PTR(raid1c34),
> >  	BTRFS_FEAT_ATTR_PTR(block_group_tree),
> > +	BTRFS_FEAT_ATTR_PTR(simple_quota),
> >  #ifdef CONFIG_BLK_DEV_ZONED
> >  	BTRFS_FEAT_ATTR_PTR(zoned),
> >  #endif
> > -- 
> > 2.41.0