On Mon, Jan 13, 2025 at 05:24:25AM +0100, Christoph Hellwig wrote: > Hi all, > > this series fixes two races in buffer refcount handling, that I've > so far not actually seen in real life, but only found while reading > through the buffer cache code to understand some of the stranger looking > locking decisions. > > One can cause a buffer about to be freed to be returned from > xfs_buf_insert. I think this one is extremely unlikely to be hit, > as it requires the buffer to not be present for the initial lookup, > but already being evicted when trying to add the new buffer. But > at least the fix is trivial. > > The second causes buffer lookups to be missed when moving to the LRU. > This might actually be able to trigger the first one, but otherwise > just means we're doing a pass through insert which will find it. > For pure lookups using xfs_buf_incore it could cause us to miss buffer > invalidation. The fix for that is bigger and has bigger implications > because it not requires all b_hold increments to be done under d_lock. Just to be clear, should this sentence say "...because it *now* requires"? > This causes more contention, but as releasing the buffer always takes > the lock it can't be too horrible. I also have a only minimally > tested series to switch it over to a lockref here: > > http://git.infradead.org/?p=users/hch/xfs.git;a=shortlog;h=refs/heads/xfs-buffer-locking Will take a look; some of those patches look familiar. ;) --D > > Diffstat: > b/fs/xfs/xfs_buf.c | 3 - > b/fs/xfs/xfs_buf.h | 4 +- > b/fs/xfs/xfs_trace.h | 10 ++--- > fs/xfs/xfs_buf.c | 93 ++++++++++++++++++++++++++------------------------- > 4 files changed, 56 insertions(+), 54 deletions(-)
