xfs
[Top] [All Lists]

[PATCH 02/11] xfs: Don't use xfs_buf_iowait in the delwri buffer code

To: xfs@xxxxxxxxxxx
Subject: [PATCH 02/11] xfs: Don't use xfs_buf_iowait in the delwri buffer code
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 25 Sep 2014 22:34:12 +1000
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1411648461-29003-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1411648461-29003-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

For the special case of delwri buffer submission and waiting, we
don't need to issue IO synchronously at all. The second pass to call
xfs_buf_iowait() can be replaced with  blocking on xfs_buf_lock() -
the buffer will be unlocked when the async IO is complete.

This formalises a sane the method of waiting for async IO - take an
extra reference, submit the IO, call xfs_buf_lock() when you want to
wait for IO completion. i.e.:

        bp = xfs_buf_find();
        xfs_buf_hold(bp);
        bp->b_flags |= XBF_ASYNC;
        xfs_buf_iosubmit(bp);
        xfs_buf_lock(bp)
        error = bp->b_error;
        ....
        xfs_buf_relse(bp);

While this is somewhat racy for gathering IO errors, none of the
code that calls xfs_buf_delwri_submit() will race against other
users of the buffers being submitted. Even if they do, we don't
really care if the error is detected by the delwri code or the user
we raced against. Either way, the error will be detected and
handled.

Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 fs/xfs/xfs_buf.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index ec65050..f1bd238 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1813,12 +1813,17 @@ __xfs_buf_delwri_submit(
        blk_start_plug(&plug);
        list_for_each_entry_safe(bp, n, io_list, b_list) {
                bp->b_flags &= ~(_XBF_DELWRI_Q | XBF_ASYNC | XBF_WRITE_FAIL);
-               bp->b_flags |= XBF_WRITE;
+               bp->b_flags |= XBF_WRITE | XBF_ASYNC;
 
-               if (!wait) {
-                       bp->b_flags |= XBF_ASYNC;
+               /*
+                * we do all Io submission async. This means if we need to wait
+                * for IO completion we need to take an extra reference so the
+                * buffer is still valid on the other side.
+                */
+               if (wait)
+                       xfs_buf_hold(bp);
+               else
                        list_del_init(&bp->b_list);
-               }
                xfs_bdstrat_cb(bp);
        }
        blk_finish_plug(&plug);
@@ -1866,7 +1871,10 @@ xfs_buf_delwri_submit(
                bp = list_first_entry(&io_list, struct xfs_buf, b_list);
 
                list_del_init(&bp->b_list);
-               error2 = xfs_buf_iowait(bp);
+
+               /* locking the buffer will wait for async IO completion. */
+               xfs_buf_lock(bp);
+               error2 = bp->b_error;
                xfs_buf_relse(bp);
                if (!error)
                        error = error2;
-- 
2.0.0

<Prev in Thread] Current Thread [Next in Thread>