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@xxxxxx>
