diff mbox series

[01/15] xfs: fix a double completion for buffers on in-memory targets

Message ID 20250106095613.847700-2-hch@lst.de (mailing list archive)
State New
Headers show
Series [01/15] xfs: fix a double completion for buffers on in-memory targets | expand

Commit Message

Christoph Hellwig Jan. 6, 2025, 9:54 a.m. UTC
__xfs_buf_submit calls xfs_buf_ioend when b_io_remaining hits zero.  For
in-memory buftargs b_io_remaining is never incremented from it's initial
value of 1, so this always happens.  Thus the extra call to xfs_buf_ioend
in _xfs_buf_ioapply causes a double completion.  Fortunately
__xfs_buf_submit is only used for synchronous reads on in-memory buftargs
due to the peculiarities of how they work, so this is mostly harmless and
just causes a little extra work to be done.

Fixes: 5076a6040ca1 ("xfs: support in-memory buffer cache targets")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Darrick J. Wong Jan. 7, 2025, 2 a.m. UTC | #1
On Mon, Jan 06, 2025 at 10:54:38AM +0100, Christoph Hellwig wrote:
> __xfs_buf_submit calls xfs_buf_ioend when b_io_remaining hits zero.  For
> in-memory buftargs b_io_remaining is never incremented from it's initial
> value of 1, so this always happens.  Thus the extra call to xfs_buf_ioend
> in _xfs_buf_ioapply causes a double completion.  Fortunately
> __xfs_buf_submit is only used for synchronous reads on in-memory buftargs
> due to the peculiarities of how they work, so this is mostly harmless and
> just causes a little extra work to be done.

Tempted to add:

Cc: <stable@vger.kernel.org> # v6.9

though I think backporting isn't strictly necessary because in-memory
buffers don't have log items, right?  If so, then we don't need to cc
stable.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> Fixes: 5076a6040ca1 ("xfs: support in-memory buffer cache targets")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index aa63b8efd782..787caf0c3254 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1656,10 +1656,8 @@ _xfs_buf_ioapply(
>  	op |= REQ_META;
>  
>  	/* in-memory targets are directly mapped, no IO required. */
> -	if (xfs_buftarg_is_mem(bp->b_target)) {
> -		xfs_buf_ioend(bp);
> +	if (xfs_buftarg_is_mem(bp->b_target))
>  		return;
> -	}
>  
>  	/*
>  	 * Walk all the vectors issuing IO on them. Set up the initial offset
> -- 
> 2.45.2
> 
>
Christoph Hellwig Jan. 7, 2025, 6:05 a.m. UTC | #2
On Mon, Jan 06, 2025 at 06:00:35PM -0800, Darrick J. Wong wrote:
> Cc: <stable@vger.kernel.org> # v6.9
> 
> though I think backporting isn't strictly necessary because in-memory
> buffers don't have log items, right?  If so, then we don't need to cc
> stable.

The stable CC is fine with me.  But as I tried to explain I think
it's nor really causing problems right now, just a double call
to read_verify.  It is a bit of a landmine, though.
diff mbox series

Patch

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index aa63b8efd782..787caf0c3254 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1656,10 +1656,8 @@  _xfs_buf_ioapply(
 	op |= REQ_META;
 
 	/* in-memory targets are directly mapped, no IO required. */
-	if (xfs_buftarg_is_mem(bp->b_target)) {
-		xfs_buf_ioend(bp);
+	if (xfs_buftarg_is_mem(bp->b_target))
 		return;
-	}
 
 	/*
 	 * Walk all the vectors issuing IO on them. Set up the initial offset