Re: [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.
> > 
> 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?

The or_softirq_pending(nr) be translated into the following three
assembly instructions(arm64 platform)：

ldr w0, [x18, #softirq_pending_offset]

by interrupt:
calling  __raise_softirq_irqoff(NET_RX_SOFTIRQ)

orr w0, w0, 1<< nr
str w0, [x18, #softirq_pending_offset] <-- still use old value, the NET_RX_SOFTIRQ maybe lose.


Thanks
Zqiang


> 
> After all changes.
> 
> Reviewed-by: Bradley Morgan <include@xxxxxxxxx>
> 
> > 
> > Signed-off-by: Zqiang <qiang.zhang@xxxxxxxxx>
> >  ---
> >  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!
>