Re: [PATCH 3/3] blk-mq: unify hctx delay_work and run_work

[Bart Van Assche <Bart.VanAssche@sandisk.com>]

On Mon, 2017-04-10 at 09:54 -0600, Jens Axboe wrote:
> @@ -1281,27 +1280,39 @@ static void blk_mq_run_work_fn(struct work_struct=
 *work)
>  	struct blk_mq_hw_ctx *hctx;
> =20
>  	hctx =3D container_of(work, struct blk_mq_hw_ctx, run_work.work);
> -	__blk_mq_run_hw_queue(hctx);
> -}
> =20
> -static void blk_mq_delay_work_fn(struct work_struct *work)
> -{
> -	struct blk_mq_hw_ctx *hctx;
> +	/*
> +	 * If we are stopped, don't run the queue. The exception is if
> +	 * BLK_MQ_S_START_ON_RUN is set. For that case, we auto-clear
> +	 * the STOPPED bit and run it.
> +	 */
> +	if (test_bit(BLK_MQ_S_STOPPED, &hctx->state)) {
> +		if (!test_bit(BLK_MQ_S_START_ON_RUN, &hctx->state))
> +			return;
> =20
> -	hctx =3D container_of(work, struct blk_mq_hw_ctx, delay_work.work);
> +		clear_bit(BLK_MQ_S_START_ON_RUN, &hctx->state);
> +		clear_bit(BLK_MQ_S_STOPPED, &hctx->state);
> +	}
> =20
> -	if (test_and_clear_bit(BLK_MQ_S_STOPPED, &hctx->state))
> -		__blk_mq_run_hw_queue(hctx);
> +	__blk_mq_run_hw_queue(hctx);
>  }
> =20
> +
>  void blk_mq_delay_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs)
>  {
>  	if (unlikely(!blk_mq_hw_queue_mapped(hctx)))
>  		return;
> =20
> +	/*
> +	 * Stop the hw queue, then modify currently delayed work.
> +	 * This should prevent us from running the queue prematurely.
> +	 * Mark the queue as auto-clearing STOPPED when it runs.
> +	 */
>  	blk_mq_stop_hw_queue(hctx);
> -	kblockd_schedule_delayed_work_on(blk_mq_hctx_next_cpu(hctx),
> -			&hctx->delay_work, msecs_to_jiffies(msecs));
> +	set_bit(BLK_MQ_S_START_ON_RUN, &hctx->state);
> +	kblockd_mod_delayed_work_on(blk_mq_hctx_next_cpu(hctx),
> +					&hctx->run_work,
> +					msecs_to_jiffies(msecs));
>  }
>  EXPORT_SYMBOL(blk_mq_delay_queue);

Hello Jens,

Is it possible for a block driver to call blk_mq_delay_queue() while
blk_mq_delay_work_fn() is running? Can that result in BLK_MQ_S_STOPPED
and=A0BLK_MQ_S_START_ON_RUN being checked by=A0blk_mq_delay_work_fn() after
blk_mq_delay_queue() has set BLK_MQ_S_STOPPED and before it has set
BLK_MQ_S_START_ON_RUN?

Thanks,

Bart.=