block, bfq: move update of waker and woken list to queue freeing

Since commit 13a857a4c4e8 ("block, bfq: detect wakers and
unconditionally inject their I/O"), every bfq_queue has a pointer to a
waker bfq_queue and a list of the bfq_queues it may wake. In this
respect, when a bfq_queue, say Q, remains with no I/O source attached
to it, Q cannot be woken by any other bfq_queue, and cannot wake any
other bfq_queue. Then Q must be removed from the woken list of its
possible waker bfq_queue, and all bfq_queues in the woken list of Q
must stop having a waker bfq_queue.

Q remains with no I/O source in two cases: when the last process
associated with Q exits or when such a process gets associated with a
different bfq_queue. Unfortunately, commit 13a857a4c4e8 ("block, bfq:
detect wakers and unconditionally inject their I/O") performed the
above updates only in the first case.

This commit fixes this bug by moving these updates to when Q gets
freed. This is a simple and safe way to handle all cases, as both the
above events, process exit and re-association, lead to Q being freed
soon, and because dangling references would come out only after Q gets
freed (if no update were performed).

Fixes: 13a857a4c4e8 ("block, bfq: detect wakers and unconditionally inject their I/O")
Reported-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index b200965..5f47750 100644 (file)
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -4765,6 +4765,8 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx)
  */
 void bfq_put_queue(struct bfq_queue *bfqq)
 {
+       struct bfq_queue *item;
+       struct hlist_node *n;
 #ifdef CONFIG_BFQ_GROUP_IOSCHED
        struct bfq_group *bfqg = bfqq_group(bfqq);
 #endif
@@ -4809,6 +4811,33 @@ void bfq_put_queue(struct bfq_queue *bfqq)
                        bfqq->bfqd->burst_size--;
        }
 
+       /*
+        * bfqq does not exist any longer, so it cannot be woken by
+        * any other queue, and cannot wake any other queue. Then bfqq
+        * must be removed from the woken list of its possible waker
+        * queue, and all queues in the woken list of bfqq must stop
+        * having a waker queue. Strictly speaking, these updates
+        * should be performed when bfqq remains with no I/O source
+        * attached to it, which happens before bfqq gets freed. In
+        * particular, this happens when the last process associated
+        * with bfqq exits or gets associated with a different
+        * queue. However, both events lead to bfqq being freed soon,
+        * and dangling references would come out only after bfqq gets
+        * freed. So these updates are done here, as a simple and safe
+        * way to handle all cases.
+        */
+       /* remove bfqq from woken list */
+       if (!hlist_unhashed(&bfqq->woken_list_node))
+               hlist_del_init(&bfqq->woken_list_node);
+
+       /* reset waker for all queues in woken list */
+       hlist_for_each_entry_safe(item, n, &bfqq->woken_list,
+                                 woken_list_node) {
+               item->waker_bfqq = NULL;
+               bfq_clear_bfqq_has_waker(item);
+               hlist_del_init(&item->woken_list_node);
+       }
+
        if (bfqq->bfqd && bfqq->bfqd->last_completed_rq_bfqq == bfqq)
                bfqq->bfqd->last_completed_rq_bfqq = NULL;
 
@@ -4839,9 +4868,6 @@ static void bfq_put_cooperator(struct bfq_queue *bfqq)
 
 static void bfq_exit_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq)
 {
-       struct bfq_queue *item;
-       struct hlist_node *n;
-
        if (bfqq == bfqd->in_service_queue) {
                __bfq_bfqq_expire(bfqd, bfqq, BFQQE_BUDGET_TIMEOUT);
                bfq_schedule_dispatch(bfqd);
@@ -4851,18 +4877,6 @@ static void bfq_exit_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq)
 
        bfq_put_cooperator(bfqq);
 
-       /* remove bfqq from woken list */
-       if (!hlist_unhashed(&bfqq->woken_list_node))
-               hlist_del_init(&bfqq->woken_list_node);
-
-       /* reset waker for all queues in woken list */
-       hlist_for_each_entry_safe(item, n, &bfqq->woken_list,
-                                 woken_list_node) {
-               item->waker_bfqq = NULL;
-               bfq_clear_bfqq_has_waker(item);
-               hlist_del_init(&item->woken_list_node);
-       }
-
        bfq_put_queue(bfqq); /* release process reference */
 }