On Wed, Apr 28, 2021 at 09:11:53AM +0200, Daniel Wagner wrote:
> Hi,
>
> On Tue, Apr 27, 2021 at 07:47:41PM -0300, Luis Claudio R. Goncalves wrote:
> > On Tue, Apr 27, 2021 at 06:10:09PM -0400, Joe Korty wrote:
> > > Balance local_irq_{disable,enable} usage in irq_forced_thread_fn
> > >
> > > Re: 0152-genirq-Allow-disabling-of-softirq-processing-in-irq-.patch
> > >
> > > In 4.9.263-rt177, irq_forced_thread_fn has potentially unbalanced calls to
> > > local_irq_disable ... local_irq_enable. This is probably not intentional.
> > >
> > > I am not absolutely sure what the proper fix is. Attached is an example
> > > of what that might look like.
> > >
> > > [ Issue detected via compiler warning, using a sufficiently advanced gcc ]
> > >
> > > Signed-off-by: Joe Korty <joe.korty@xxxxxxxxxxxxxxxxx>
> >
> > Joe, I fell a bit behind on v4.9-rt and intend to get up to date this
> > weekend. I am now satisfied with my current backport of the futex changes
> > from v4.9.264 and my 96h of pi_stress testing.
> >
> > Your patch makes sense, taking v4.19-rt as reference, but I will wait
> > for comments from the wise people you listed on the Cc: before taking
> > action.
>
> I had to resolve the same merge conflict in v4.4-rt. The fix from Joe
> resolve to the same solution I've done in v4.4-rt
I will add this patch to the next v4.9-rt without the extra empty line.
Thank you both,
Luis
> >
> > Best regards,
> > Luis
> >
> > >
> > > Index: b/kernel/irq/manage.c
> > > ===================================================================
> > > --- a/kernel/irq/manage.c
> > > +++ b/kernel/irq/manage.c
> > > @@ -1035,6 +1035,9 @@ irq_forced_thread_fn(struct irq_desc *de
> > > atomic_inc(&desc->threads_handled);
> > >
> > > irq_finalize_oneshot(desc, action);
> > > +
>
> Nit there is no new line here in upstream, dd652c6ab49b ("genirq:
> Disable interrupts for force threaded handlers")
>
> > > + if (!IS_ENABLED(CONFIG_PREEMPT_RT_BASE))
> > > + local_irq_enable();
> > > /*
> > > * Interrupts which have real time requirements can be set up
> > > * to avoid softirq processing in the thread handler. This is
> > > @@ -1043,8 +1046,6 @@ irq_forced_thread_fn(struct irq_desc *de
> > > if (irq_settings_no_softirq_call(desc))
> > > _local_bh_enable();
> > > else
> > > - if (!IS_ENABLED(CONFIG_PREEMPT_RT_BASE))
> > > - local_irq_enable();
> > > local_bh_enable();
> > > return ret;
> > > }
> > ---end quoted text---
>
> Thanks,
> Daniel
---end quoted text---
