Re: [PATCH 3.19 016/175] ksoftirqd: Enable IRQs and call cond_resched() before poking RCU

[Greg Kroah-Hartman]

On Fri, May 01, 2015 at 03:00:00PM -0500, Josh Hunt wrote:
> On Wed, Mar 4, 2015 at 12:13 AM, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > 3.19-stable review patch.  If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Calvin Owens <calvinowens@xxxxxx>
> >
> > commit 28423ad283d5348793b0c45cc9b1af058e776fd6 upstream.
> >
> > While debugging an issue with excessive softirq usage, I encountered the
> > following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread
> > infrastructure"):
> >
> >     [ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]
> >
> > ...but despite this note, the patch still calls RCU with IRQs disabled.
> >
> > This seemingly innocuous change caused a significant regression in softirq
> > CPU usage on the sending side of a large TCP transfer (~1 GB/s): when
> > introducing 0.01% packet loss, the softirq usage would jump to around 25%,
> > spiking as high as 50%. Before the change, the usage would never exceed 5%.
> >
> > Moving the call to rcu_note_context_switch() after the cond_sched() call,
> > as it was originally before the hotplug patch, completely eliminated this
> > problem.
> >
> > Signed-off-by: Calvin Owens <calvinowens@xxxxxx>
> > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >
> > ---
> >  kernel/softirq.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > --- a/kernel/softirq.c
> > +++ b/kernel/softirq.c
> > @@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int c
> >                  * in the task stack here.
> >                  */
> >                 __do_softirq();
> > -               rcu_note_context_switch();
> >                 local_irq_enable();
> >                 cond_resched();
> > +
> > +               preempt_disable();
> > +               rcu_note_context_switch();
> > +               preempt_enable();
> > +
> >                 return;
> >         }
> >         local_irq_enable();
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> Sorry for the delay in noticing this, but should this be applied to
> 3.14-stable as well?

Why should it?  And odds are, if I didn't apply it there, it was either
because it didn't apply, or it broke the build.  Have you tried this out
in 3.14 to see if it does even work?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/