| Message ID | 20250106095613.847700-8-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 |
On Mon, Jan 06, 2025 at 10:54:44AM +0100, Christoph Hellwig wrote: > Split the write verification logic out of _xfs_buf_ioapply into a new > xfs_buf_verify_write helper called by xfs_buf_submit given that it isn't > about applying the I/O and doesn't really fit in with the rest of > _xfs_buf_ioapply. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Yeah, it's useful to break up this function... Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > fs/xfs/xfs_buf.c | 67 ++++++++++++++++++++++++++---------------------- > 1 file changed, 37 insertions(+), 30 deletions(-) > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > index e48d796c786b..18e830c4e990 100644 > --- a/fs/xfs/xfs_buf.c > +++ b/fs/xfs/xfs_buf.c > @@ -1615,36 +1615,6 @@ _xfs_buf_ioapply( > > if (bp->b_flags & XBF_WRITE) { > op = REQ_OP_WRITE; > - > - /* > - * Run the write verifier callback function if it exists. If > - * this function fails it will mark the buffer with an error and > - * the IO should not be dispatched. > - */ > - if (bp->b_ops) { > - bp->b_ops->verify_write(bp); > - if (bp->b_error) { > - xfs_force_shutdown(bp->b_mount, > - SHUTDOWN_CORRUPT_INCORE); > - return; > - } > - } else if (bp->b_rhash_key != XFS_BUF_DADDR_NULL) { > - struct xfs_mount *mp = bp->b_mount; > - > - /* > - * non-crc filesystems don't attach verifiers during > - * log recovery, so don't warn for such filesystems. > - */ > - if (xfs_has_crc(mp)) { > - xfs_warn(mp, > - "%s: no buf ops on daddr 0x%llx len %d", > - __func__, xfs_buf_daddr(bp), > - bp->b_length); > - xfs_hex_dump(bp->b_addr, > - XFS_CORRUPTION_DUMP_LEN); > - dump_stack(); > - } > - } > } else { > op = REQ_OP_READ; > if (bp->b_flags & XBF_READ_AHEAD) > @@ -1693,6 +1663,36 @@ xfs_buf_iowait( > return bp->b_error; > } > > +/* > + * Run the write verifier callback function if it exists. If this fails, mark > + * the buffer with an error and do not dispatch the I/O. > + */ > +static bool > +xfs_buf_verify_write( > + struct xfs_buf *bp) > +{ > + if (bp->b_ops) { > + bp->b_ops->verify_write(bp); > + if (bp->b_error) > + return false; > + } else if (bp->b_rhash_key != XFS_BUF_DADDR_NULL) { > + /* > + * Non-crc filesystems don't attach verifiers during log > + * recovery, so don't warn for such filesystems. > + */ > + if (xfs_has_crc(bp->b_mount)) { > + xfs_warn(bp->b_mount, > + "%s: no buf ops on daddr 0x%llx len %d", > + __func__, xfs_buf_daddr(bp), > + bp->b_length); > + xfs_hex_dump(bp->b_addr, XFS_CORRUPTION_DUMP_LEN); > + dump_stack(); > + } > + } > + > + return true; > +} > + > /* > * Buffer I/O submission path, read or write. Asynchronous submission transfers > * the buffer lock ownership and the current reference to the IO. It is not > @@ -1751,8 +1751,15 @@ xfs_buf_submit( > atomic_set(&bp->b_io_remaining, 1); > if (bp->b_flags & XBF_ASYNC) > xfs_buf_ioacct_inc(bp); > + > + if ((bp->b_flags & XBF_WRITE) && !xfs_buf_verify_write(bp)) { > + xfs_force_shutdown(bp->b_mount, SHUTDOWN_CORRUPT_INCORE); > + goto done; > + } > + > _xfs_buf_ioapply(bp); > > +done: > /* > * If _xfs_buf_ioapply failed, we can get back here with only the IO > * reference we took above. If we drop it to zero, run completion so > -- > 2.45.2 > >
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index e48d796c786b..18e830c4e990 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -1615,36 +1615,6 @@ _xfs_buf_ioapply( if (bp->b_flags & XBF_WRITE) { op = REQ_OP_WRITE; - - /* - * Run the write verifier callback function if it exists. If - * this function fails it will mark the buffer with an error and - * the IO should not be dispatched. - */ - if (bp->b_ops) { - bp->b_ops->verify_write(bp); - if (bp->b_error) { - xfs_force_shutdown(bp->b_mount, - SHUTDOWN_CORRUPT_INCORE); - return; - } - } else if (bp->b_rhash_key != XFS_BUF_DADDR_NULL) { - struct xfs_mount *mp = bp->b_mount; - - /* - * non-crc filesystems don't attach verifiers during - * log recovery, so don't warn for such filesystems. - */ - if (xfs_has_crc(mp)) { - xfs_warn(mp, - "%s: no buf ops on daddr 0x%llx len %d", - __func__, xfs_buf_daddr(bp), - bp->b_length); - xfs_hex_dump(bp->b_addr, - XFS_CORRUPTION_DUMP_LEN); - dump_stack(); - } - } } else { op = REQ_OP_READ; if (bp->b_flags & XBF_READ_AHEAD) @@ -1693,6 +1663,36 @@ xfs_buf_iowait( return bp->b_error; } +/* + * Run the write verifier callback function if it exists. If this fails, mark + * the buffer with an error and do not dispatch the I/O. + */ +static bool +xfs_buf_verify_write( + struct xfs_buf *bp) +{ + if (bp->b_ops) { + bp->b_ops->verify_write(bp); + if (bp->b_error) + return false; + } else if (bp->b_rhash_key != XFS_BUF_DADDR_NULL) { + /* + * Non-crc filesystems don't attach verifiers during log + * recovery, so don't warn for such filesystems. + */ + if (xfs_has_crc(bp->b_mount)) { + xfs_warn(bp->b_mount, + "%s: no buf ops on daddr 0x%llx len %d", + __func__, xfs_buf_daddr(bp), + bp->b_length); + xfs_hex_dump(bp->b_addr, XFS_CORRUPTION_DUMP_LEN); + dump_stack(); + } + } + + return true; +} + /* * Buffer I/O submission path, read or write. Asynchronous submission transfers * the buffer lock ownership and the current reference to the IO. It is not @@ -1751,8 +1751,15 @@ xfs_buf_submit( atomic_set(&bp->b_io_remaining, 1); if (bp->b_flags & XBF_ASYNC) xfs_buf_ioacct_inc(bp); + + if ((bp->b_flags & XBF_WRITE) && !xfs_buf_verify_write(bp)) { + xfs_force_shutdown(bp->b_mount, SHUTDOWN_CORRUPT_INCORE); + goto done; + } + _xfs_buf_ioapply(bp); +done: /* * If _xfs_buf_ioapply failed, we can get back here with only the IO * reference we took above. If we drop it to zero, run completion so
Split the write verification logic out of _xfs_buf_ioapply into a new xfs_buf_verify_write helper called by xfs_buf_submit given that it isn't about applying the I/O and doesn't really fit in with the rest of _xfs_buf_ioapply. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_buf.c | 67 ++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 30 deletions(-)