[PATCH] workqueue: Use raise_softirq() to trigger softirq in irq_work handler

[PATCH] workqueue: Use raise_softirq() to trigger softirq in irq_work handler

[Zqiang]

The bh_pool_kick_normal() and bh_pool_kick_highpri() are registered via
init_irq_work() without IRQ_WORK_HARD_IRQ flag. On PREEMPT_RT, such
work items are processed by the per-CPU irq_workd kthread in preemptible
task context with IRQs enabled. However calling raise_softirq_irqoff()
requires IRQS must be disabled, This commit therefore replace
raise_softirq_irqoff() with raise_softirq() in irq_work handler.

Signed-off-by: Zqiang <qiang.zhang@linux.dev>
---
 kernel/workqueue.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 78068ae8f28a..0bb978df0200 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -7923,12 +7923,12 @@ static inline void wq_watchdog_init(void) { }
 
 static void bh_pool_kick_normal(struct irq_work *irq_work)
 {
-	raise_softirq_irqoff(TASKLET_SOFTIRQ);
+	raise_softirq(TASKLET_SOFTIRQ);
 }
 
 static void bh_pool_kick_highpri(struct irq_work *irq_work)
 {
-	raise_softirq_irqoff(HI_SOFTIRQ);
+	raise_softirq(HI_SOFTIRQ);
 }
 
 static void __init restrict_unbound_cpumask(const char *name, const struct cpumask *mask)
-- 
2.17.1

Re: [PATCH] workqueue: Use raise_softirq() to trigger softirq in irq_work handler

[Bradley Morgan]

> The bh_pool_kick_normal() and bh_pool_kick_highpri() are registered via
> init_irq_work() without IRQ_WORK_HARD_IRQ flag. On PREEMPT_RT, such
> work items are processed by the per-CPU irq_workd kthread in preemptible
> task context with IRQs enabled. However calling raise_softirq_irqoff()
> requires IRQS must be disabled, This commit therefore replace
> raise_softirq_irqoff() with raise_softirq() in irq_work handler.

Hi zhang!

Could you add

Fixes: 4cb1ef64609f ("workqueue: Implement BH workqueues to eventually replace tasklets")

?

Also, if you would like this to go to stable, could you write a small
part of the description about the user-space effects and why this is
bad?


After all changes.


Reviewed-by: Bradley Morgan <include@grrlz.net>


> Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> ---
>  kernel/workqueue.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 78068ae8f28a..0bb978df0200 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -7923,12 +7923,12 @@ static inline void wq_watchdog_init(void) { }
>  
>  static void bh_pool_kick_normal(struct irq_work *irq_work)
>  {
> -	raise_softirq_irqoff(TASKLET_SOFTIRQ);
> +	raise_softirq(TASKLET_SOFTIRQ);
>  }
>  
>  static void bh_pool_kick_highpri(struct irq_work *irq_work)
>  {
> -	raise_softirq_irqoff(HI_SOFTIRQ);
> +	raise_softirq(HI_SOFTIRQ);
>  }
>  
>  static void __init restrict_unbound_cpumask(const char *name, const struct cpumask *mask)
> -- 
> 2.17.1

Thanks!

Re: [PATCH] workqueue: Use raise_softirq() to trigger softirq in irq_work handler

[Tejun Heo]

Hello,

Applied to wq/for-7.1-fixes with the description reworded and the
following tags added:

  Fixes: 2f34d7337d98 ("workqueue: Fix queue_work_on() with BH workqueues")
  Cc: stable@vger.kernel.org # v6.9+

The Fixes target differs from the suggested 4cb1ef64609f. There, the
raises ran under pool->lock with IRQs disabled. The irq_work handlers
were added later by 2f34d7337d98 which moved the remote-CPU raise out of
kick_pool(). Also picked up Bradley's Reviewed-by.

Thanks.

-- 
tejun