[PATCH REPOST] blk-mq: Don't disable preemption around __blk_mq_run_hw_queue().

[PATCH REPOST] blk-mq: Don't disable preemption around __blk_mq_run_hw_queue().

[Sebastian Andrzej Siewior]

__blk_mq_delay_run_hw_queue() disables preemption to get a stable
current CPU number and then invokes __blk_mq_run_hw_queue() if the CPU
number is part the mask.

__blk_mq_run_hw_queue() acquires a spin_lock_t which is a sleeping lock
on PREEMPT_RT and can't be acquired with disabled preemption.

If it is important that the current CPU matches the requested CPU mask
and that the context does not migrate to another CPU while
__blk_mq_run_hw_queue() is invoked then it possible to achieve this by
disabling migration and keeping the context preemptible.

Disable only migration while testing the CPU mask and invoking
__blk_mq_run_hw_queue().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://lore.kernel.org/r/YnQHqx/5+54jd+U+@linutronix.de
Link: https://lore.kernel.org/r/YqISXf6GAQeWqcR+@linutronix.de
---
 block/blk-mq.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2083,14 +2083,14 @@ static void __blk_mq_delay_run_hw_queue(
 		return;
 
 	if (!async && !(hctx->flags & BLK_MQ_F_BLOCKING)) {
-		int cpu = get_cpu();
-		if (cpumask_test_cpu(cpu, hctx->cpumask)) {
+		migrate_disable();
+		if (cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask)) {
 			__blk_mq_run_hw_queue(hctx);
-			put_cpu();
+			migrate_enable();
 			return;
 		}
 
-		put_cpu();
+		migrate_enable();
 	}
 
 	kblockd_mod_delayed_work_on(blk_mq_hctx_next_cpu(hctx), &hctx->run_work,

Re: [PATCH REPOST] blk-mq: Don't disable preemption around __blk_mq_run_hw_queue().

[Ming Lei]

On Tue, Jun 21, 2022 at 09:39:19AM +0200, Sebastian Andrzej Siewior wrote:
> __blk_mq_delay_run_hw_queue() disables preemption to get a stable
> current CPU number and then invokes __blk_mq_run_hw_queue() if the CPU
> number is part the mask.
> 
> __blk_mq_run_hw_queue() acquires a spin_lock_t which is a sleeping lock
> on PREEMPT_RT and can't be acquired with disabled preemption.
> 
> If it is important that the current CPU matches the requested CPU mask
> and that the context does not migrate to another CPU while
> __blk_mq_run_hw_queue() is invoked then it possible to achieve this by
> disabling migration and keeping the context preemptible.
> 
> Disable only migration while testing the CPU mask and invoking
> __blk_mq_run_hw_queue().
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Link: https://lore.kernel.org/r/YnQHqx/5+54jd+U+@linutronix.de
> Link: https://lore.kernel.org/r/YqISXf6GAQeWqcR+@linutronix.de
> ---
>  block/blk-mq.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2083,14 +2083,14 @@ static void __blk_mq_delay_run_hw_queue(
>  		return;
>  
>  	if (!async && !(hctx->flags & BLK_MQ_F_BLOCKING)) {
> -		int cpu = get_cpu();
> -		if (cpumask_test_cpu(cpu, hctx->cpumask)) {
> +		migrate_disable();
> +		if (cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask)) {
>  			__blk_mq_run_hw_queue(hctx);
> -			put_cpu();
> +			migrate_enable();

I think you can replace it with raw_smp_processor_id() directly without
disabling migration.

Both async run queue and direct issue can run on cpu not on hctx->cpumask,
so it is fine for sync run queue too. 

Thanks,
Ming

[PATCH v2] blk-mq: Don't disable preemption around __blk_mq_run_hw_queue().

[Sebastian Andrzej Siewior]

__blk_mq_delay_run_hw_queue() disables preemption to get a stable
current CPU number and then invokes __blk_mq_run_hw_queue() if the CPU
number is part the mask.

__blk_mq_run_hw_queue() acquires a spin_lock_t which is a sleeping lock
on PREEMPT_RT and can't be acquired with disabled preemption.

It is not required for correctness to invoke __blk_mq_run_hw_queue() on
a CPU matching hctx->cpumask. Both (async and direct requests) can run
on a CPU not matching hctx->cpumask.

The CPU mask without disabling preemption and invoking
__blk_mq_run_hw_queue().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2:
  - Drop migrate_disable() as per Ming Lei.

 block/blk-mq.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2085,14 +2085,10 @@ static void __blk_mq_delay_run_hw_queue(
 		return;
 
 	if (!async && !(hctx->flags & BLK_MQ_F_BLOCKING)) {
-		int cpu = get_cpu();
-		if (cpumask_test_cpu(cpu, hctx->cpumask)) {
+		if (cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask)) {
 			__blk_mq_run_hw_queue(hctx);
-			put_cpu();
 			return;
 		}
-
-		put_cpu();
 	}
 
 	kblockd_mod_delayed_work_on(blk_mq_hctx_next_cpu(hctx), &hctx->run_work,

Re: [PATCH v2] blk-mq: Don't disable preemption around __blk_mq_run_hw_queue().

[Ming Lei]

On Wed, Jun 22, 2022 at 10:25:54AM +0200, Sebastian Andrzej Siewior wrote:
> __blk_mq_delay_run_hw_queue() disables preemption to get a stable
> current CPU number and then invokes __blk_mq_run_hw_queue() if the CPU
> number is part the mask.
> 
> __blk_mq_run_hw_queue() acquires a spin_lock_t which is a sleeping lock
> on PREEMPT_RT and can't be acquired with disabled preemption.
> 
> It is not required for correctness to invoke __blk_mq_run_hw_queue() on
> a CPU matching hctx->cpumask. Both (async and direct requests) can run
> on a CPU not matching hctx->cpumask.
> 
> The CPU mask without disabling preemption and invoking
> __blk_mq_run_hw_queue().
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Looks fine:

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Thanks,
Ming

Re: [PATCH v2] blk-mq: Don't disable preemption around __blk_mq_run_hw_queue().

[Jens Axboe]

On Wed, 22 Jun 2022 10:25:54 +0200, Sebastian Andrzej Siewior wrote:
> __blk_mq_delay_run_hw_queue() disables preemption to get a stable
> current CPU number and then invokes __blk_mq_run_hw_queue() if the CPU
> number is part the mask.
> 
> __blk_mq_run_hw_queue() acquires a spin_lock_t which is a sleeping lock
> on PREEMPT_RT and can't be acquired with disabled preemption.
> 
> [...]

Applied, thanks!

[1/1] blk-mq: Don't disable preemption around __blk_mq_run_hw_queue().
      commit: c9198d784fa93d447afe8e4627dfe205f0ce5ec8

Best regards,
-- 
Jens Axboe