Re: [PATCH] xfs: remove xfs_buf_cache.bc_lock

Christoph Hellwig
2025-01-28T05:06:19Z

On Tue, Jan 28, 2025 at 07:19:11AM +1100, Dave Chinner wrote:
> Ok, so now we can get racing inserts, which means this can find
> the buffer that has just been inserted by another thread in this
> same function. Or, indeed, and xfs_buf_lookup() call.

Yes.

> What prevents
> those racing tasks from using this buffer before the task that
> inserted it can use it?
>
> I think that the the buffer lock being initialised to "held" and
> b_hold being initialised to 1 make this all work correctly,

Exactly, the buffer is inserted with the b_sema held and b_hold
initializes 1, aka locked and held.

> but
> comments that explicitly spell out why RCU inserts are safe
> (both in xfs_buf_alloc() for the init values and here) would be
> appreciated.

Sure.

> > struct xfs_buf_cache {
> > - spinlock_t bc_lock;
> > struct rhashtable bc_hash;
> > };
>
> At this point, the struct xfs_buf_cache structure can go away,
> right? (separate patch and all that...)

Yes. And in fact I think the per-pag hash should also go away, as with
the per-bucket locking there is no point in it. I've had this patch in
my testing runs for a while, which I think is where we should be