On Mon, Jan 06, 2025 at 10:54:41AM +0100, Christoph Hellwig wrote:
> There is no good reason to pass a bool argument to wait for a buffer when
> the callers that want that can easily just wait themselves.
>
> This means the wait moves out of the extra hold of the buffer, but as the
> callers of synchronous buffer I/O need to hold a reference anyway that is
> perfectly fine.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
That's much cleaner...
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_buf.c | 36 +++++++++++++++++-------------------
> 1 file changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 1927655fed13..a3484421a6d8 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -52,14 +52,8 @@ struct kmem_cache *xfs_buf_cache;
> * b_lock (trylock due to inversion)
> */
>
> -static int __xfs_buf_submit(struct xfs_buf *bp, bool wait);
> -
> -static inline int
> -xfs_buf_submit(
> - struct xfs_buf *bp)
> -{
> - return __xfs_buf_submit(bp, !(bp->b_flags & XBF_ASYNC));
> -}
> +static int xfs_buf_submit(struct xfs_buf *bp);
> +static int xfs_buf_iowait(struct xfs_buf *bp);
>
> static inline bool xfs_buf_is_uncached(struct xfs_buf *bp)
> {
> @@ -797,13 +791,18 @@ _xfs_buf_read(
> struct xfs_buf *bp,
> xfs_buf_flags_t flags)
> {
> + int error;
> +
> ASSERT(!(flags & XBF_WRITE));
> ASSERT(bp->b_maps[0].bm_bn != XFS_BUF_DADDR_NULL);
>
> bp->b_flags &= ~(XBF_WRITE | XBF_ASYNC | XBF_READ_AHEAD | XBF_DONE);
> bp->b_flags |= flags & (XBF_READ | XBF_ASYNC | XBF_READ_AHEAD);
>
> - return xfs_buf_submit(bp);
> + error = xfs_buf_submit(bp);
> + if (!error && !(flags & XBF_ASYNC))
> + error = xfs_buf_iowait(bp);
> + return error;
> }
>
> /*
> @@ -978,9 +977,10 @@ xfs_buf_read_uncached(
> bp->b_flags |= XBF_READ;
> bp->b_ops = ops;
>
> - xfs_buf_submit(bp);
> - if (bp->b_error) {
> - error = bp->b_error;
> + error = xfs_buf_submit(bp);
> + if (!error)
> + error = xfs_buf_iowait(bp);
> + if (error) {
> xfs_buf_relse(bp);
> return error;
> }
> @@ -1483,6 +1483,8 @@ xfs_bwrite(
> XBF_DONE);
>
> error = xfs_buf_submit(bp);
> + if (!error)
> + error = xfs_buf_iowait(bp);
> if (error)
> xfs_force_shutdown(bp->b_mount, SHUTDOWN_META_IO_ERROR);
> return error;
> @@ -1698,9 +1700,8 @@ xfs_buf_iowait(
> * holds an additional reference itself.
> */
> static int
> -__xfs_buf_submit(
> - struct xfs_buf *bp,
> - bool wait)
> +xfs_buf_submit(
> + struct xfs_buf *bp)
> {
> int error = 0;
>
> @@ -1764,9 +1765,6 @@ __xfs_buf_submit(
> xfs_buf_ioend_async(bp);
> }
>
> - if (wait)
> - error = xfs_buf_iowait(bp);
> -
> /*
> * Release the hold that keeps the buffer referenced for the entire
> * I/O. Note that if the buffer is async, it is not safe to reference
> @@ -2322,7 +2320,7 @@ xfs_buf_delwri_submit_buffers(
> bp->b_flags |= XBF_ASYNC;
> xfs_buf_list_del(bp);
> }
> - __xfs_buf_submit(bp, false);
> + xfs_buf_submit(bp);
> }
> blk_finish_plug(&plug);
>
> --
> 2.45.2
>
>
