Re: [PATCH] block: fix io hung by block throttle

[Junxiao Bi]

On 4/17/21 3:10 AM, Hillf Danton wrote:

--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -260,19 +260,17 @@ void rq_qos_wait(struct rq_wait *rqw, void *private_data,
                .cb = acquire_inflight_cb,
                .private_data = private_data,
        };
-       bool has_sleeper;
  
-	has_sleeper = wq_has_sleeper(&rqw->wait);
-       if (!has_sleeper && acquire_inflight_cb(rqw, private_data))
+       if (!wq_has_sleeper(&rqw->wait)
+               && acquire_inflight_cb(rqw, private_data))
                return;
  
  	prepare_to_wait_exclusive(&rqw->wait, &data.wq, TASK_UNINTERRUPTIBLE);
-       has_sleeper = !wq_has_single_sleeper(&rqw->wait);
        do {
                /* The memory barrier in set_task_state saves us here. */
                if (data.got_token)
                        break;
-               if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) {
+               if (acquire_inflight_cb(rqw, private_data)) {
                        finish_wait(&rqw->wait, &data.wq);
Simply removing !has_sleeper is not enough if it is mandatory before
acquire_inflight_cb() without adding something like a mutex to sieve the
concurrent sleepers out, see below.

--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -260,19 +260,18 @@ void rq_qos_wait(struct rq_wait *rqw, vo
                .cb = acquire_inflight_cb,
                .private_data = private_data,
        };
-       bool has_sleeper;
  
-	has_sleeper = wq_has_sleeper(&rqw->wait);
-       if (!has_sleeper && acquire_inflight_cb(rqw, private_data))
-               return;
+       mutex_lock(&rqw->mutex);
+
+       if (acquire_inflight_cb(rqw, private_data))

This function is to increase atomic variable rq_wait->inflight. What's 
the mutex for?

Thanks,

Junxiao.

+               goto out;
  
  	prepare_to_wait_exclusive(&rqw->wait, &data.wq, TASK_UNINTERRUPTIBLE);
-       has_sleeper = !wq_has_single_sleeper(&rqw->wait);
        do {
                /* The memory barrier in set_task_state saves us here. */
                if (data.got_token)
                        break;
-               if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) {
+               if (acquire_inflight_cb(rqw, private_data)) {
                        finish_wait(&rqw->wait, &data.wq);
  
  			/*
@@ -286,10 +285,11 @@ void rq_qos_wait(struct rq_wait *rqw, vo
                        break;
                }
                io_schedule();
-               has_sleeper = true;
                set_current_state(TASK_UNINTERRUPTIBLE);
        } while (1);
        finish_wait(&rqw->wait, &data.wq);
+out:
+       mutex_unlock(&rqw->mutex);
  }