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 going: http://git.infradead.org/?p=users/hch/xfs.git;a=commitdiff;h=890cd2cd255710ee5d3408bc60792b9cdad3adfb