* fix a buffer lookup against removal race
@ 2026-05-15 13:31 Christoph Hellwig
2026-05-15 13:31 ` [PATCH] xfs: " Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2026-05-15 13:31 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: Andrey Albershteyn, linux-xfs
Hi all,
this fixes a bug introduced in the buffer caching changes in 7.0.
Andrey originally found this when testing the fsverity series, and Carlos
came up with a reproducer using xfstests generic/579 with the fsverity
bits stripped out on upstream. I'll look at adding this as a separate
test as well.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] xfs: fix a buffer lookup against removal race
2026-05-15 13:31 fix a buffer lookup against removal race Christoph Hellwig
@ 2026-05-15 13:31 ` Christoph Hellwig
2026-05-15 15:34 ` Carlos Maiolino
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Christoph Hellwig @ 2026-05-15 13:31 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: Andrey Albershteyn, linux-xfs
When a buffer is freed either by LRU eviction or because it is unset,
the lockref is marked as dead instantly, which prevents the buffer from
being used after finding it in the buffer hash in xfs_buf_lookup and
xfs_buf_find_insert. But the latter will then not add the new buffer to
the hash because it already found an existing buffer.
Fix this using in two places: Remove the buffer from the hash before
marking the lockref dead so that that no buffer with a dead lockref can
be found in the hash, but if we find one in xfs_buf_find_insert due to
store reordering, handle this case correctly instead of returning an
unhashed buffer.
Fixes: 67fe4303972e ("xfs: don't keep a reference for buffers on the LRU")
Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
Reported-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_buf.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 580d40a5ee57..a095a5c0a01f 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -472,6 +472,7 @@ xfs_buf_find_insert(
/* The new buffer keeps the perag reference until it is freed. */
new_bp->b_pag = pag;
+retry:
rcu_read_lock();
bp = rhashtable_lookup_get_insert_fast(&btp->bt_hash,
&new_bp->b_rhash_head, xfs_buf_hash_params);
@@ -480,8 +481,15 @@ xfs_buf_find_insert(
error = PTR_ERR(bp);
goto out_free_buf;
}
- if (bp && lockref_get_not_dead(&bp->b_lockref)) {
- /* found an existing buffer */
+ if (bp) {
+ /*
+ * If there is an existing buffer with a dead lockref, retry
+ * until the new buffer is added or usable buffer is found.
+ */
+ if (!lockref_get_not_dead(&bp->b_lockref)) {
+ rcu_read_unlock();
+ goto retry;
+ }
rcu_read_unlock();
error = xfs_buf_find_lock(bp, flags);
if (error)
@@ -820,15 +828,20 @@ xfs_buf_destroy(
ASSERT(__lockref_is_dead(&bp->b_lockref));
ASSERT(!(bp->b_flags & _XBF_DELWRI_Q));
+ if (bp->b_pag)
+ xfs_perag_put(bp->b_pag);
+ xfs_buf_free(bp);
+}
+
+static inline void
+xfs_buf_kill(
+ struct xfs_buf *bp)
+{
if (!xfs_buf_is_uncached(bp)) {
rhashtable_remove_fast(&bp->b_target->bt_hash,
&bp->b_rhash_head, xfs_buf_hash_params);
-
- if (bp->b_pag)
- xfs_perag_put(bp->b_pag);
}
-
- xfs_buf_free(bp);
+ lockref_mark_dead(&bp->b_lockref);
}
/*
@@ -851,7 +864,7 @@ xfs_buf_rele(
return;
kill:
- lockref_mark_dead(&bp->b_lockref);
+ xfs_buf_kill(bp);
list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru);
spin_unlock(&bp->b_lockref.lock);
@@ -1433,7 +1446,7 @@ xfs_buftarg_drain_rele(
return LRU_SKIP;
}
- lockref_mark_dead(&bp->b_lockref);
+ xfs_buf_kill(bp);
list_lru_isolate_move(lru, item, dispose);
spin_unlock(&bp->b_lockref.lock);
return LRU_REMOVED;
@@ -1545,7 +1558,7 @@ xfs_buftarg_isolate(
return LRU_ROTATE;
}
- lockref_mark_dead(&bp->b_lockref);
+ xfs_buf_kill(bp);
list_lru_isolate_move(lru, item, dispose);
spin_unlock(&bp->b_lockref.lock);
return LRU_REMOVED;
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: fix a buffer lookup against removal race
2026-05-15 13:31 ` [PATCH] xfs: " Christoph Hellwig
@ 2026-05-15 15:34 ` Carlos Maiolino
2026-05-15 21:59 ` Dave Chinner
2026-05-16 12:55 ` Andrey Albershteyn
2 siblings, 0 replies; 10+ messages in thread
From: Carlos Maiolino @ 2026-05-15 15:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs
On Fri, May 15, 2026 at 03:31:58PM +0200, Christoph Hellwig wrote:
> When a buffer is freed either by LRU eviction or because it is unset,
> the lockref is marked as dead instantly, which prevents the buffer from
> being used after finding it in the buffer hash in xfs_buf_lookup and
> xfs_buf_find_insert. But the latter will then not add the new buffer to
> the hash because it already found an existing buffer.
>
> Fix this using in two places: Remove the buffer from the hash before
> marking the lockref dead so that that no buffer with a dead lockref can
> be found in the hash, but if we find one in xfs_buf_find_insert due to
> store reordering, handle this case correctly instead of returning an
> unhashed buffer.
>
> Fixes: 67fe4303972e ("xfs: don't keep a reference for buffers on the LRU")
> Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
> Reported-by: Carlos Maiolino <cem@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> fs/xfs/xfs_buf.c | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 580d40a5ee57..a095a5c0a01f 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -472,6 +472,7 @@ xfs_buf_find_insert(
> /* The new buffer keeps the perag reference until it is freed. */
> new_bp->b_pag = pag;
>
> +retry:
> rcu_read_lock();
> bp = rhashtable_lookup_get_insert_fast(&btp->bt_hash,
> &new_bp->b_rhash_head, xfs_buf_hash_params);
> @@ -480,8 +481,15 @@ xfs_buf_find_insert(
> error = PTR_ERR(bp);
> goto out_free_buf;
> }
> - if (bp && lockref_get_not_dead(&bp->b_lockref)) {
> - /* found an existing buffer */
> + if (bp) {
> + /*
> + * If there is an existing buffer with a dead lockref, retry
> + * until the new buffer is added or usable buffer is found.
> + */
> + if (!lockref_get_not_dead(&bp->b_lockref)) {
> + rcu_read_unlock();
> + goto retry;
> + }
> rcu_read_unlock();
> error = xfs_buf_find_lock(bp, flags);
> if (error)
> @@ -820,15 +828,20 @@ xfs_buf_destroy(
> ASSERT(__lockref_is_dead(&bp->b_lockref));
> ASSERT(!(bp->b_flags & _XBF_DELWRI_Q));
>
> + if (bp->b_pag)
> + xfs_perag_put(bp->b_pag);
> + xfs_buf_free(bp);
> +}
> +
> +static inline void
> +xfs_buf_kill(
> + struct xfs_buf *bp)
> +{
> if (!xfs_buf_is_uncached(bp)) {
> rhashtable_remove_fast(&bp->b_target->bt_hash,
> &bp->b_rhash_head, xfs_buf_hash_params);
> -
> - if (bp->b_pag)
> - xfs_perag_put(bp->b_pag);
> }
> -
> - xfs_buf_free(bp);
> + lockref_mark_dead(&bp->b_lockref);
> }
>
> /*
> @@ -851,7 +864,7 @@ xfs_buf_rele(
> return;
>
> kill:
> - lockref_mark_dead(&bp->b_lockref);
> + xfs_buf_kill(bp);
> list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru);
> spin_unlock(&bp->b_lockref.lock);
>
> @@ -1433,7 +1446,7 @@ xfs_buftarg_drain_rele(
> return LRU_SKIP;
> }
>
> - lockref_mark_dead(&bp->b_lockref);
> + xfs_buf_kill(bp);
> list_lru_isolate_move(lru, item, dispose);
> spin_unlock(&bp->b_lockref.lock);
> return LRU_REMOVED;
> @@ -1545,7 +1558,7 @@ xfs_buftarg_isolate(
> return LRU_ROTATE;
> }
>
> - lockref_mark_dead(&bp->b_lockref);
> + xfs_buf_kill(bp);
> list_lru_isolate_move(lru, item, dispose);
> spin_unlock(&bp->b_lockref.lock);
> return LRU_REMOVED;
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: fix a buffer lookup against removal race
2026-05-15 13:31 ` [PATCH] xfs: " Christoph Hellwig
2026-05-15 15:34 ` Carlos Maiolino
@ 2026-05-15 21:59 ` Dave Chinner
2026-05-18 5:44 ` Christoph Hellwig
2026-05-16 12:55 ` Andrey Albershteyn
2 siblings, 1 reply; 10+ messages in thread
From: Dave Chinner @ 2026-05-15 21:59 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Carlos Maiolino, Andrey Albershteyn, linux-xfs
On Fri, May 15, 2026 at 03:31:58PM +0200, Christoph Hellwig wrote:
> When a buffer is freed either by LRU eviction or because it is unset,
> the lockref is marked as dead instantly, which prevents the buffer from
> being used after finding it in the buffer hash in xfs_buf_lookup and
> xfs_buf_find_insert. But the latter will then not add the new buffer to
> the hash because it already found an existing buffer.
>
> Fix this using in two places: Remove the buffer from the hash before
> marking the lockref dead so that that no buffer with a dead lockref can
> be found in the hash, but if we find one in xfs_buf_find_insert due to
> store reordering, handle this case correctly instead of returning an
> unhashed buffer.
>
> Fixes: 67fe4303972e ("xfs: don't keep a reference for buffers on the LRU")
> Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
> Reported-by: Carlos Maiolino <cem@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_buf.c | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 580d40a5ee57..a095a5c0a01f 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -472,6 +472,7 @@ xfs_buf_find_insert(
> /* The new buffer keeps the perag reference until it is freed. */
> new_bp->b_pag = pag;
>
> +retry:
> rcu_read_lock();
> bp = rhashtable_lookup_get_insert_fast(&btp->bt_hash,
> &new_bp->b_rhash_head, xfs_buf_hash_params);
> @@ -480,8 +481,15 @@ xfs_buf_find_insert(
> error = PTR_ERR(bp);
> goto out_free_buf;
> }
> - if (bp && lockref_get_not_dead(&bp->b_lockref)) {
> - /* found an existing buffer */
> + if (bp) {
> + /*
> + * If there is an existing buffer with a dead lockref, retry
> + * until the new buffer is added or usable buffer is found.
> + */
> + if (!lockref_get_not_dead(&bp->b_lockref)) {
> + rcu_read_unlock();
> + goto retry;
> + }
Like the inode cache, there probably should be a delay here rather
than spinning hard. There is no guarantee that the object actually
appears removed from the cache until the RCU grace period expires,
though typically races that find objects being removed are much
shorter duration than that.
Also, is it safe to run lockref_get_not_dead() whilst some other
thread is racing to get lockref.lock and calls lockref_mark_dead()
on it?
> rcu_read_unlock();
> error = xfs_buf_find_lock(bp, flags);
> if (error)
> @@ -820,15 +828,20 @@ xfs_buf_destroy(
> ASSERT(__lockref_is_dead(&bp->b_lockref));
> ASSERT(!(bp->b_flags & _XBF_DELWRI_Q));
>
> + if (bp->b_pag)
> + xfs_perag_put(bp->b_pag);
> + xfs_buf_free(bp);
> +}
> +
> +static inline void
> +xfs_buf_kill(
> + struct xfs_buf *bp)
> +{
> if (!xfs_buf_is_uncached(bp)) {
> rhashtable_remove_fast(&bp->b_target->bt_hash,
> &bp->b_rhash_head, xfs_buf_hash_params);
> -
> - if (bp->b_pag)
> - xfs_perag_put(bp->b_pag);
> }
> -
> - xfs_buf_free(bp);
> + lockref_mark_dead(&bp->b_lockref);
> }
That'll cause issues. RCU algorithms require the object to be marked
dead before it is removed from the index so that RCU lookup races
that find it after removal (i.e. during the RCU grace period) see
the object as dead, not as a valid buffer (think RT preemption
between remove and mark dead).
-Dave.
--
Dave Chinner
dgc@kernel.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: fix a buffer lookup against removal race
2026-05-15 13:31 ` [PATCH] xfs: " Christoph Hellwig
2026-05-15 15:34 ` Carlos Maiolino
2026-05-15 21:59 ` Dave Chinner
@ 2026-05-16 12:55 ` Andrey Albershteyn
2 siblings, 0 replies; 10+ messages in thread
From: Andrey Albershteyn @ 2026-05-16 12:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs
On 2026-05-15 15:31:58, Christoph Hellwig wrote:
> When a buffer is freed either by LRU eviction or because it is unset,
> the lockref is marked as dead instantly, which prevents the buffer from
> being used after finding it in the buffer hash in xfs_buf_lookup and
> xfs_buf_find_insert. But the latter will then not add the new buffer to
> the hash because it already found an existing buffer.
>
> Fix this using in two places: Remove the buffer from the hash before
> marking the lockref dead so that that no buffer with a dead lockref can
> be found in the hash, but if we find one in xfs_buf_find_insert due to
> store reordering, handle this case correctly instead of returning an
> unhashed buffer.
>
> Fixes: 67fe4303972e ("xfs: don't keep a reference for buffers on the LRU")
> Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
> Reported-by: Carlos Maiolino <cem@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Thanks for quick fix!
The issues is gone now, tested on v7.1-rc3 and v7.1-rc3 with
fsverity.
Tested-by: Andrey Albershteyn <aalbersh@kernel.org>
--
- Andrey
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: fix a buffer lookup against removal race
2026-05-15 21:59 ` Dave Chinner
@ 2026-05-18 5:44 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2026-05-18 5:44 UTC (permalink / raw)
To: Dave Chinner
Cc: Christoph Hellwig, Carlos Maiolino, Andrey Albershteyn, linux-xfs
On Sat, May 16, 2026 at 07:59:17AM +1000, Dave Chinner wrote:
> > + if (bp) {
> > + /*
> > + * If there is an existing buffer with a dead lockref, retry
> > + * until the new buffer is added or usable buffer is found.
> > + */
> > + if (!lockref_get_not_dead(&bp->b_lockref)) {
> > + rcu_read_unlock();
> > + goto retry;
> > + }
>
> Like the inode cache, there probably should be a delay here rather
> than spinning hard. There is no guarantee that the object actually
> appears removed from the cache until the RCU grace period expires,
> though typically races that find objects being removed are much
> shorter duration than that.
I don't think in the current version we need it because it is
basically imposisble to hit. But with your comment below fixed
we do need it, so I'll add it.
> Also, is it safe to run lockref_get_not_dead() whilst some other
> thread is racing to get lockref.lock and calls lockref_mark_dead()
> on it?
Yes, it is specifically designed for that: the lockref idea is that you
can do fast path increment/decrements using atomics that are serialized
as if you'd always take the lock around manual opeations on the count
field. The ability to mark it dead under the lock and synchronize
against atomic increments one of the most important aspects of that.
> That'll cause issues. RCU algorithms require the object to be marked
> dead before it is removed from the index so that RCU lookup races
> that find it after removal (i.e. during the RCU grace period) see
> the object as dead, not as a valid buffer (think RT preemption
> between remove and mark dead).
Yes, this should be switched around. And with that the retry loop
above becomes more likely and needs the cpu_relax().
I wrote a version doing this and ran it through test over the weekend,
which I'll post soon.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] xfs: fix a buffer lookup against removal race
2026-05-18 6:02 fix a buffer lookup against removal race v2 Christoph Hellwig
@ 2026-05-18 6:02 ` Christoph Hellwig
2026-05-20 9:11 ` Carlos Maiolino
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Christoph Hellwig @ 2026-05-18 6:02 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: Andrey Albershteyn, Dave Chinner, linux-xfs
When a buffer is freed either by LRU eviction or because it is unset,
the lockref is marked as dead instantly, which prevents the buffer from
being used after finding it in the buffer hash in xfs_buf_lookup and
xfs_buf_find_insert. But the latter will then not add the new buffer to
the hash because it already found an existing buffer.
Fix this using in two places: Remove the buffer from the hash before
marking the lockref dead so that that no buffer with a dead lockref can
be found in the hash, but if we find one in xfs_buf_find_insert due to
store reordering, handle this case correctly instead of returning an
unhashed buffer.
Fixes: 67fe4303972e ("xfs: don't keep a reference for buffers on the LRU")
Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
Reported-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_buf.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 580d40a5ee57..0cea458f1353 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -472,6 +472,7 @@ xfs_buf_find_insert(
/* The new buffer keeps the perag reference until it is freed. */
new_bp->b_pag = pag;
+retry:
rcu_read_lock();
bp = rhashtable_lookup_get_insert_fast(&btp->bt_hash,
&new_bp->b_rhash_head, xfs_buf_hash_params);
@@ -480,8 +481,16 @@ xfs_buf_find_insert(
error = PTR_ERR(bp);
goto out_free_buf;
}
- if (bp && lockref_get_not_dead(&bp->b_lockref)) {
- /* found an existing buffer */
+ if (bp) {
+ /*
+ * If there is an existing buffer with a dead lockref, retry
+ * until the new buffer is added, or a usable buffer is found.
+ */
+ if (!lockref_get_not_dead(&bp->b_lockref)) {
+ rcu_read_unlock();
+ cpu_relax();
+ goto retry;
+ }
rcu_read_unlock();
error = xfs_buf_find_lock(bp, flags);
if (error)
@@ -820,15 +829,20 @@ xfs_buf_destroy(
ASSERT(__lockref_is_dead(&bp->b_lockref));
ASSERT(!(bp->b_flags & _XBF_DELWRI_Q));
+ if (bp->b_pag)
+ xfs_perag_put(bp->b_pag);
+ xfs_buf_free(bp);
+}
+
+static inline void
+xfs_buf_kill(
+ struct xfs_buf *bp)
+{
+ lockref_mark_dead(&bp->b_lockref);
if (!xfs_buf_is_uncached(bp)) {
rhashtable_remove_fast(&bp->b_target->bt_hash,
&bp->b_rhash_head, xfs_buf_hash_params);
-
- if (bp->b_pag)
- xfs_perag_put(bp->b_pag);
}
-
- xfs_buf_free(bp);
}
/*
@@ -851,7 +865,7 @@ xfs_buf_rele(
return;
kill:
- lockref_mark_dead(&bp->b_lockref);
+ xfs_buf_kill(bp);
list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru);
spin_unlock(&bp->b_lockref.lock);
@@ -1433,7 +1447,7 @@ xfs_buftarg_drain_rele(
return LRU_SKIP;
}
- lockref_mark_dead(&bp->b_lockref);
+ xfs_buf_kill(bp);
list_lru_isolate_move(lru, item, dispose);
spin_unlock(&bp->b_lockref.lock);
return LRU_REMOVED;
@@ -1545,7 +1559,7 @@ xfs_buftarg_isolate(
return LRU_ROTATE;
}
- lockref_mark_dead(&bp->b_lockref);
+ xfs_buf_kill(bp);
list_lru_isolate_move(lru, item, dispose);
spin_unlock(&bp->b_lockref.lock);
return LRU_REMOVED;
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: fix a buffer lookup against removal race
2026-05-18 6:02 ` [PATCH] xfs: fix a buffer lookup against removal race Christoph Hellwig
@ 2026-05-20 9:11 ` Carlos Maiolino
2026-05-21 11:38 ` Andrey Albershteyn
2026-05-26 9:56 ` Carlos Maiolino
2 siblings, 0 replies; 10+ messages in thread
From: Carlos Maiolino @ 2026-05-20 9:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andrey Albershteyn, Dave Chinner, linux-xfs
On Mon, May 18, 2026 at 08:02:05AM +0200, Christoph Hellwig wrote:
> When a buffer is freed either by LRU eviction or because it is unset,
> the lockref is marked as dead instantly, which prevents the buffer from
> being used after finding it in the buffer hash in xfs_buf_lookup and
> xfs_buf_find_insert. But the latter will then not add the new buffer to
> the hash because it already found an existing buffer.
>
> Fix this using in two places: Remove the buffer from the hash before
> marking the lockref dead so that that no buffer with a dead lockref can
> be found in the hash, but if we find one in xfs_buf_find_insert due to
> store reordering, handle this case correctly instead of returning an
> unhashed buffer.
>
> Fixes: 67fe4303972e ("xfs: don't keep a reference for buffers on the LRU")
> Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
> Reported-by: Carlos Maiolino <cem@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
That looks good to me.
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> fs/xfs/xfs_buf.c | 34 ++++++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 580d40a5ee57..0cea458f1353 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -472,6 +472,7 @@ xfs_buf_find_insert(
> /* The new buffer keeps the perag reference until it is freed. */
> new_bp->b_pag = pag;
>
> +retry:
> rcu_read_lock();
> bp = rhashtable_lookup_get_insert_fast(&btp->bt_hash,
> &new_bp->b_rhash_head, xfs_buf_hash_params);
> @@ -480,8 +481,16 @@ xfs_buf_find_insert(
> error = PTR_ERR(bp);
> goto out_free_buf;
> }
> - if (bp && lockref_get_not_dead(&bp->b_lockref)) {
> - /* found an existing buffer */
> + if (bp) {
> + /*
> + * If there is an existing buffer with a dead lockref, retry
> + * until the new buffer is added, or a usable buffer is found.
> + */
> + if (!lockref_get_not_dead(&bp->b_lockref)) {
> + rcu_read_unlock();
> + cpu_relax();
> + goto retry;
> + }
> rcu_read_unlock();
> error = xfs_buf_find_lock(bp, flags);
> if (error)
> @@ -820,15 +829,20 @@ xfs_buf_destroy(
> ASSERT(__lockref_is_dead(&bp->b_lockref));
> ASSERT(!(bp->b_flags & _XBF_DELWRI_Q));
>
> + if (bp->b_pag)
> + xfs_perag_put(bp->b_pag);
> + xfs_buf_free(bp);
> +}
> +
> +static inline void
> +xfs_buf_kill(
> + struct xfs_buf *bp)
> +{
> + lockref_mark_dead(&bp->b_lockref);
> if (!xfs_buf_is_uncached(bp)) {
> rhashtable_remove_fast(&bp->b_target->bt_hash,
> &bp->b_rhash_head, xfs_buf_hash_params);
> -
> - if (bp->b_pag)
> - xfs_perag_put(bp->b_pag);
> }
> -
> - xfs_buf_free(bp);
> }
>
> /*
> @@ -851,7 +865,7 @@ xfs_buf_rele(
> return;
>
> kill:
> - lockref_mark_dead(&bp->b_lockref);
> + xfs_buf_kill(bp);
> list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru);
> spin_unlock(&bp->b_lockref.lock);
>
> @@ -1433,7 +1447,7 @@ xfs_buftarg_drain_rele(
> return LRU_SKIP;
> }
>
> - lockref_mark_dead(&bp->b_lockref);
> + xfs_buf_kill(bp);
> list_lru_isolate_move(lru, item, dispose);
> spin_unlock(&bp->b_lockref.lock);
> return LRU_REMOVED;
> @@ -1545,7 +1559,7 @@ xfs_buftarg_isolate(
> return LRU_ROTATE;
> }
>
> - lockref_mark_dead(&bp->b_lockref);
> + xfs_buf_kill(bp);
> list_lru_isolate_move(lru, item, dispose);
> spin_unlock(&bp->b_lockref.lock);
> return LRU_REMOVED;
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: fix a buffer lookup against removal race
2026-05-18 6:02 ` [PATCH] xfs: fix a buffer lookup against removal race Christoph Hellwig
2026-05-20 9:11 ` Carlos Maiolino
@ 2026-05-21 11:38 ` Andrey Albershteyn
2026-05-26 9:56 ` Carlos Maiolino
2 siblings, 0 replies; 10+ messages in thread
From: Andrey Albershteyn @ 2026-05-21 11:38 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Carlos Maiolino, Andrey Albershteyn, Dave Chinner, linux-xfs
On 2026-05-18 08:02:05, Christoph Hellwig wrote:
> When a buffer is freed either by LRU eviction or because it is unset,
> the lockref is marked as dead instantly, which prevents the buffer from
> being used after finding it in the buffer hash in xfs_buf_lookup and
> xfs_buf_find_insert. But the latter will then not add the new buffer to
> the hash because it already found an existing buffer.
>
> Fix this using in two places: Remove the buffer from the hash before
> marking the lockref dead so that that no buffer with a dead lockref can
> be found in the hash, but if we find one in xfs_buf_find_insert due to
> store reordering, handle this case correctly instead of returning an
> unhashed buffer.
>
> Fixes: 67fe4303972e ("xfs: don't keep a reference for buffers on the LRU")
> Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
> Reported-by: Carlos Maiolino <cem@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good to me
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
--
- Andrey
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: fix a buffer lookup against removal race
2026-05-18 6:02 ` [PATCH] xfs: fix a buffer lookup against removal race Christoph Hellwig
2026-05-20 9:11 ` Carlos Maiolino
2026-05-21 11:38 ` Andrey Albershteyn
@ 2026-05-26 9:56 ` Carlos Maiolino
2 siblings, 0 replies; 10+ messages in thread
From: Carlos Maiolino @ 2026-05-26 9:56 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andrey Albershteyn, Dave Chinner, linux-xfs
On Mon, 18 May 2026 08:02:05 +0200, Christoph Hellwig wrote:
> When a buffer is freed either by LRU eviction or because it is unset,
> the lockref is marked as dead instantly, which prevents the buffer from
> being used after finding it in the buffer hash in xfs_buf_lookup and
> xfs_buf_find_insert. But the latter will then not add the new buffer to
> the hash because it already found an existing buffer.
>
> Fix this using in two places: Remove the buffer from the hash before
> marking the lockref dead so that that no buffer with a dead lockref can
> be found in the hash, but if we find one in xfs_buf_find_insert due to
> store reordering, handle this case correctly instead of returning an
> unhashed buffer.
>
> [...]
Applied to for-next, thanks!
[1/1] xfs: fix a buffer lookup against removal race
commit: c69439a891ccb37ede5d68539636337c6bd92fab
Best regards,
--
Carlos Maiolino <cem@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-05-26 9:56 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-15 13:31 fix a buffer lookup against removal race Christoph Hellwig
2026-05-15 13:31 ` [PATCH] xfs: " Christoph Hellwig
2026-05-15 15:34 ` Carlos Maiolino
2026-05-15 21:59 ` Dave Chinner
2026-05-18 5:44 ` Christoph Hellwig
2026-05-16 12:55 ` Andrey Albershteyn
-- strict thread matches above, loose matches on Subject: below --
2026-05-18 6:02 fix a buffer lookup against removal race v2 Christoph Hellwig
2026-05-18 6:02 ` [PATCH] xfs: fix a buffer lookup against removal race Christoph Hellwig
2026-05-20 9:11 ` Carlos Maiolino
2026-05-21 11:38 ` Andrey Albershteyn
2026-05-26 9:56 ` Carlos Maiolino
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox