Re: [PATCH 02/10] xfs: move RT inode locking out of __xfs_bunmapi

Darrick J. Wong
2024-02-23T16:46:56Z

On Fri, Feb 23, 2024 at 05:37:37PM +0100, Christoph Hellwig wrote:
> On Fri, Feb 23, 2024 at 08:34:48AM -0800, Darrick J. Wong wrote:
> > I don't really like using transaction flags to record lock state.
> >
> > Would this be cleaner if we tracked this in struct xfs_rtalloc_args, and
> > had an xfs_rtalloc_acquire(mp, &args, XFS_ILOCK_{SHARED,EXCL}) method
> > that would set that up for us?
>
> Urgg, that would be pretty ugly, and not work for the new locking in the
> extfree item once you add that, which has the same weird layering
> violation.

<nod> Dave occasionally talks about turning allocation into a log intent
so that we could decouple both of these things. I think the difficulty
there is that you need a way for the allocator to notice that a
particular "free" extent is actually "busy". That's trivial for the
bnobt allocator, but nonexistent for rtalloc.

> The only "sane" way out would be to always use a deferred item, which we
> should be doing for anything using new RT features, but we can't really
> do that for legacy file systems without forcing a log incompat flag.
> So while I don't particularly like the transaction flag it seems like
> the least evil solution.

I had thought about doing that for rtgroups=1 filesystems. :)

--D