Re: fix buffer refcount races

["Darrick J. Wong" <djwong@kernel.org>]

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(-)