From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DC93B4C9542 for ; Fri, 15 May 2026 15:34:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778859293; cv=none; b=B0qIAvjC29DHstJDn6XB/49op4QkLhcJrpVDtgoX9Zooowka2Wt8wVD4jk/0w4337Spp7Rp3y4bKFXRjhGd2iYIc/nY3Qcbo5g4bhZfrq8qs49JJmk411ioOPNZ50DykycBj0CnoiBgetTP01eV17BOI9OH7sj8alOX/2Xg6EsY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778859293; c=relaxed/simple; bh=fTwFnM2V6S7H83/SbFDh4CdoSy1gyXo2pg6Ep2agp34=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hpMsCP5OMcDI2D3yqYk6x53YIVubPF0GWgosX1fzXQI9cNHFvjsOZoCy66A5bF52kw7EXeBStZcE7M7wpKvyGKUO9EKrcZVzyyks5EgoVg2HUCg5gNno1gI7zAlB00nYNqL4E36Uut+X+gZWvUZQ+6iELB04nrr8+49bhf6RdR4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=PWqvDthx; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="PWqvDthx" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4F224C2BCB0; Fri, 15 May 2026 15:34:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778859292; bh=fTwFnM2V6S7H83/SbFDh4CdoSy1gyXo2pg6Ep2agp34=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PWqvDthx4cmaN8MhwiXD8C4z2BCeTWmD6h7WOdPENJNWIiFyQ5YsfxxJcvC48wfDd j//p5hzOk5WHDm+XjTBz3tKXnGQ9Aa7AMOzZpszxN2mrIOehSE0YuF920zIpn8kJje EgXIyDJ81Ss7bi9BQtUh4pYJG8XtYwFyWKftejk8hWi0mrTYB9TyDaqEVafptQgN+n STB71SCRUKvXDTTbhCwa+JUcUIn+YdXr1VBPOn/jzoqHNQ4lwW8SNN2uk1u2FBmUQt nMrS0yyvBK4PKgWUHl3cUDkn3B1w4Gx9Ez47Si5SzRXsz2SvXV7FXP7sR3WaGrlhxD Ytml8cKmj98Lw== Date: Fri, 15 May 2026 17:34:48 +0200 From: Carlos Maiolino To: Christoph Hellwig Cc: Andrey Albershteyn , linux-xfs@vger.kernel.org Subject: Re: [PATCH] xfs: fix a buffer lookup against removal race Message-ID: References: <20260515133212.4039831-1-hch@lst.de> <20260515133212.4039831-2-hch@lst.de> Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260515133212.4039831-2-hch@lst.de> 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 > Reported-by: Carlos Maiolino > Signed-off-by: Christoph Hellwig > --- Reviewed-by: Carlos Maiolino > 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 >