block: Simplify bsg complete all
authorPeter Zijlstra <peterz@infradead.org>
Tue, 3 Feb 2015 11:55:31 +0000 (12:55 +0100)
committerJens Axboe <axboe@fb.com>
Wed, 4 Feb 2015 16:57:52 +0000 (09:57 -0700)
It took me a few tries to figure out what this code did; lets rewrite
it into a more regular form.

The thing that makes this one 'special' is the BSG_F_BLOCK flag, if
that is not set we're not supposed/allowed to block and should spin
wait for completion.

The (new) io_wait_event() will never see a false condition in case of
the spinning and we will therefore not block.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Jens Axboe <axboe@fb.com>
block/bsg.c
include/linux/wait.h

index 276e869e686cbbdb39112f19b163e353f0ddc3ef..d214e929ce1855bb653808f03482580b45b5344e 100644 (file)
@@ -136,42 +136,6 @@ static inline struct hlist_head *bsg_dev_idx_hash(int index)
        return &bsg_device_list[index & (BSG_LIST_ARRAY_SIZE - 1)];
 }
 
-static int bsg_io_schedule(struct bsg_device *bd)
-{
-       DEFINE_WAIT(wait);
-       int ret = 0;
-
-       spin_lock_irq(&bd->lock);
-
-       BUG_ON(bd->done_cmds > bd->queued_cmds);
-
-       /*
-        * -ENOSPC or -ENODATA?  I'm going for -ENODATA, meaning "I have no
-        * work to do", even though we return -ENOSPC after this same test
-        * during bsg_write() -- there, it means our buffer can't have more
-        * bsg_commands added to it, thus has no space left.
-        */
-       if (bd->done_cmds == bd->queued_cmds) {
-               ret = -ENODATA;
-               goto unlock;
-       }
-
-       if (!test_bit(BSG_F_BLOCK, &bd->flags)) {
-               ret = -EAGAIN;
-               goto unlock;
-       }
-
-       prepare_to_wait(&bd->wq_done, &wait, TASK_UNINTERRUPTIBLE);
-       spin_unlock_irq(&bd->lock);
-       io_schedule();
-       finish_wait(&bd->wq_done, &wait);
-
-       return ret;
-unlock:
-       spin_unlock_irq(&bd->lock);
-       return ret;
-}
-
 static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,
                                struct sg_io_v4 *hdr, struct bsg_device *bd,
                                fmode_t has_write_perm)
@@ -482,6 +446,30 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq, struct sg_io_v4 *hdr,
        return ret;
 }
 
+static bool bsg_complete(struct bsg_device *bd)
+{
+       bool ret = false;
+       bool spin;
+
+       do {
+               spin_lock_irq(&bd->lock);
+
+               BUG_ON(bd->done_cmds > bd->queued_cmds);
+
+               /*
+                * All commands consumed.
+                */
+               if (bd->done_cmds == bd->queued_cmds)
+                       ret = true;
+
+               spin = !test_bit(BSG_F_BLOCK, &bd->flags);
+
+               spin_unlock_irq(&bd->lock);
+       } while (!ret && spin);
+
+       return ret;
+}
+
 static int bsg_complete_all_commands(struct bsg_device *bd)
 {
        struct bsg_command *bc;
@@ -492,17 +480,7 @@ static int bsg_complete_all_commands(struct bsg_device *bd)
        /*
         * wait for all commands to complete
         */
-       ret = 0;
-       do {
-               ret = bsg_io_schedule(bd);
-               /*
-                * look for -ENODATA specifically -- we'll sometimes get
-                * -ERESTARTSYS when we've taken a signal, but we can't
-                * return until we're done freeing the queue, so ignore
-                * it.  The signal will get handled when we're done freeing
-                * the bsg_device.
-                */
-       } while (ret != -ENODATA);
+       io_wait_event(bd->wq_done, bsg_complete(bd));
 
        /*
         * discard done commands
index 2232ed16635ae0929c97f62e1c8c1b09109f0649..71fc1d31e48df339feb1ff1911aa9c4aa9c8e351 100644 (file)
@@ -267,6 +267,21 @@ do {                                                                       \
        __wait_event(wq, condition);                                    \
 } while (0)
 
+#define __io_wait_event(wq, condition)                                 \
+       (void)___wait_event(wq, condition, TASK_UNINTERRUPTIBLE, 0, 0,  \
+                           io_schedule())
+
+/*
+ * io_wait_event() -- like wait_event() but with io_schedule()
+ */
+#define io_wait_event(wq, condition)                                   \
+do {                                                                   \
+       might_sleep();                                                  \
+       if (condition)                                                  \
+               break;                                                  \
+       __io_wait_event(wq, condition);                                 \
+} while (0)
+
 #define __wait_event_freezable(wq, condition)                          \
        ___wait_event(wq, condition, TASK_INTERRUPTIBLE, 0, 0,          \
                            schedule(); try_to_freeze())