Re: [PATCH 1/4] xfs: buffer pins need to hold a buffer reference

[Christoph Hellwig <hch@infradead.org>]

On Wed, May 17, 2023 at 10:04:46AM +1000, Dave Chinner wrote:
> To fix this, we need to ensure that buffer existence extends beyond
> the BLI reference count checks and until the unpin processing is
> complete. This implies that a buffer pin operation must also take a
> buffer reference to ensure that the buffer cannot be freed until the
> buffer unpin processing is complete.

Yeah.  I wonder why we haven't done this from the very beginning..

> +	 /*
> +	  * Nothing to do but drop the buffer pin reference if the BLI is
> +	  * still active
> +	  */

Nit: this block comment is indentented by an extra space.

> +	if (!freed) {
> +		xfs_buf_rele(bp);
>  		return;
> +	}
>  
>  	if (stale) {

Nit: this is the only use of the stale variable now, so we might
as well just drop it.

>  		ASSERT(bip->bli_flags & XFS_BLI_STALE);

.. which then also clearly shows this ASSERT is pointless now.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>