Re: [PATCH 1/4] xfs: don't keep a reference for buffers on the LRU

[Dave Chinner <dgc@kernel.org>]

On Tue, Mar 17, 2026 at 02:40:52PM +0100, Christoph Hellwig wrote:
> @@ -942,10 +887,22 @@ xfs_buf_rele(
>  	struct xfs_buf		*bp)
>  {
>  	trace_xfs_buf_rele(bp, _RET_IP_);
> -	if (xfs_buf_is_uncached(bp))
> -		xfs_buf_rele_uncached(bp);
> -	else
> -		xfs_buf_rele_cached(bp);
> +
> +	spin_lock(&bp->b_lock);
> +	if (!--bp->b_hold) {
> +		if (xfs_buf_is_uncached(bp) || !atomic_read(&bp->b_lru_ref))
> +			goto kill;
> +		list_lru_add_obj(&bp->b_target->bt_lru, &bp->b_lru);
> +	}
> +	spin_unlock(&bp->b_lock);
> +	return;
> +
> +kill:
> +	bp->b_hold = -1;
> +	list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru);
> +	spin_unlock(&bp->b_lock);
> +
> +	xfs_buf_destroy(bp);
>  }

The jump with the lock held is kinda nasty. The lock scope can be
straight-lined by inverting the logic like so:

	spin_lock(&bp->b_lock);
	if (--bp->b_hold) {
		spin_unlock(&bp->b_lock);
		return;
	}

	if (!xfs_buf_is_uncached(bp) && atomic_read(&bp->b_lru_ref)) {
		list_lru_add_obj(&bp->b_target->bt_lru, &bp->b_lru);
		spin_unlock(&bp->b_lock);
		return;
	}

	/* time to die */
	bp->b_hold = -1;
	list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru);
	spin_unlock(&bp->b_lock);

	xfs_buf_destroy(bp);

Otherwise the change looks OK.

-Dave.

-- 
Dave Chinner
dgc@kernel.org