From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751911AbcELUHJ (ORCPT ); Thu, 12 May 2016 16:07:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51407 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751843AbcELUHH (ORCPT ); Thu, 12 May 2016 16:07:07 -0400 Message-ID: <1463083620.5281.1.camel@redhat.com> Subject: Re: [RFC PATCH 0/2] net: threadable napi poll loop From: Paolo Abeni To: Eric Dumazet Cc: Peter Zijlstra , Hannes Frederic Sowa , Eric Dumazet , netdev , "David S. Miller" , Jiri Pirko , Daniel Borkmann , Alexei Starovoitov , Alexander Duyck , Tom Herbert , Ingo Molnar , Rik van Riel , LKML , "Paul E. McKenney" Date: Thu, 12 May 2016 22:07:00 +0200 In-Reply-To: <1463003804.23934.154.camel@edumazet-glaptop3.roam.corp.google.com> References: <1462890590.23934.68.camel@edumazet-glaptop3.roam.corp.google.com> <90f3db8c-c30c-b204-576a-454939ac93ce@stressinduktion.org> <94f323a9-515e-4d75-cac8-ef0214f0499e@stressinduktion.org> <1462920697.23934.113.camel@edumazet-glaptop3.roam.corp.google.com> <20160511065527.GD3193@twins.programming.kicks-ass.net> <1463003804.23934.154.camel@edumazet-glaptop3.roam.corp.google.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 12 May 2016 20:07:05 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2016-05-11 at 14:56 -0700, Eric Dumazet wrote: > On Wed, 2016-05-11 at 08:55 +0200, Peter Zijlstra wrote: > > On Tue, May 10, 2016 at 03:51:37PM -0700, Eric Dumazet wrote: > > > diff --git a/kernel/softirq.c b/kernel/softirq.c > > > index 17caf4b63342..22463217e3cf 100644 > > > --- a/kernel/softirq.c > > > +++ b/kernel/softirq.c > > > @@ -56,6 +56,7 @@ EXPORT_SYMBOL(irq_stat); > > > static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp; > > > > > > DEFINE_PER_CPU(struct task_struct *, ksoftirqd); > > > +DEFINE_PER_CPU(bool, ksoftirqd_scheduled); > > > > > > const char * const softirq_to_name[NR_SOFTIRQS] = { > > > "HI", "TIMER", "NET_TX", "NET_RX", "BLOCK", "BLOCK_IOPOLL", > > > @@ -73,8 +74,10 @@ static void wakeup_softirqd(void) > > > /* Interrupts are disabled: no need to stop preemption */ > > > struct task_struct *tsk = __this_cpu_read(ksoftirqd); > > > > > > - if (tsk && tsk->state != TASK_RUNNING) > > > + if (tsk && tsk->state != TASK_RUNNING) { > > > + __this_cpu_write(ksoftirqd_scheduled, true); > > > wake_up_process(tsk); > > > > Since we're already looking at tsk->state, and the wake_up_process() > > ensures the thing becomes TASK_RUNNING, you could add: > > > > static inline bool ksoftirqd_running(void) > > { > > return __this_cpu_read(ksoftirqd)->state == TASK_RUNNING; here something like: struct task_struct *tsk = __this_cpu_read(ksoftirqd); return tsk && (tsk->state == TASK_RUNNING); is needed since __this_cpu_read(ksoftirqd) can be NULL on boot. > > } > > Indeed, and the patch looks quite simple now ;) > > diff --git a/kernel/softirq.c b/kernel/softirq.c > index 17caf4b63342d7839528f367b283a386413b0362..23c364485d03618773c385d943c0ef39f5931d09 100644 > --- a/kernel/softirq.c > +++ b/kernel/softirq.c > @@ -57,6 +57,11 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp > > DEFINE_PER_CPU(struct task_struct *, ksoftirqd); > > +static inline bool ksoftirqd_running(void) > +{ > + return __this_cpu_read(ksoftirqd)->state == TASK_RUNNING; > +} > + > const char * const softirq_to_name[NR_SOFTIRQS] = { > "HI", "TIMER", "NET_TX", "NET_RX", "BLOCK", "BLOCK_IOPOLL", > "TASKLET", "SCHED", "HRTIMER", "RCU" > @@ -313,7 +318,7 @@ asmlinkage __visible void do_softirq(void) > > pending = local_softirq_pending(); > > - if (pending) > + if (pending && !ksoftirqd_running()) > do_softirq_own_stack(); > > local_irq_restore(flags); > @@ -340,6 +345,9 @@ void irq_enter(void) > > static inline void invoke_softirq(void) > { > + if (ksoftirqd_running()) > + return; > + > if (!force_irqthreads) { > #ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK > /* > >