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

[Junxiao Bi <junxiao.bi@xxxxxxxxxx>]

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);
>   }