[RFC PATCH 1/4] softirq: Limit vector to a single iteration on IRQ tail

From: Frederic Weisbecker <frederic@kernel.org>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>,
	Levin Alexander <alexander.levin@verizon.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Mauro Carvalho Chehab <mchehab@s-opensource.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
	Wanpeng Li <wanpeng.li@hotmail.com>,
	Dmitry Safonov <dima@arista.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Paolo Abeni <pabeni@redhat.com>, Radu Rendec <rrendec@arista.com>,
	Ingo Molnar <mingo@kernel.org>,
	Stanislaw Gruszka <sgruszka@redhat.com>,
	Rik van Riel <riel@redhat.com>,
	Eric Dumazet <edumazet@google.com>,
	David Miller <davem@davemloft.net>
Subject: [RFC PATCH 1/4] softirq: Limit vector to a single iteration on IRQ tail
Date: Fri, 19 Jan 2018 16:46:11 +0100	[thread overview]
Message-ID: <1516376774-24076-2-git-send-email-frederic@kernel.org> (raw)
In-Reply-To: <1516376774-24076-1-git-send-email-frederic@kernel.org>

Softirqs are currently allowed to execute as much pending work as needed
as long as it doesn't overrun 10 iterations or 2 ms. The rest of the
pending work is deferred to ksoftirqd beyond that limit.

In fact 2 ms is a very high threshold that shouldn't be reachable after
only 10 iterations. And just counting iterations doesn't help diagnose
which vector forces softirqs to be deferred to a thread at the expense
of other vectors that could need quick service. Instead of blindly
kicking ksoftirqd when the softirq load becomes too high, we rather
want to offload the culprit vector only and let the other vectors to
be handled on IRQ tail as usual.

To achieve this, we rather keep track of which vectors have run on
past iterations and kick them off to threading if they get re-enqueued.
We assume that a vector only needs one iteration on normal loads while
re-enqueuing is a matter of stress, so this should be a simple yet
reliable way to detect which vector needs to be queued off in a task.

For now, vectors that get re-enqueued trigger ksoftirqd but they are
going to be handled by per-vector workqueues on subsequent patches.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Dmitry Safonov <dima@arista.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Miller <davem@davemloft.net>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Levin Alexander <alexander.levin@verizon.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Radu Rendec <rrendec@arista.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 kernel/softirq.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index 2f5e87f..c8c6841 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -190,22 +190,6 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 }
 EXPORT_SYMBOL(__local_bh_enable_ip);
 
-/*
- * We restart softirq processing for at most MAX_SOFTIRQ_RESTART times,
- * but break the loop if need_resched() is set or after 2 ms.
- * The MAX_SOFTIRQ_TIME provides a nice upper bound in most cases, but in
- * certain cases, such as stop_machine(), jiffies may cease to
- * increment and so we need the MAX_SOFTIRQ_RESTART limit as
- * well to make sure we eventually return from this method.
- *
- * These limits have been established via experimentation.
- * The two things to balance is latency against fairness -
- * we want to handle softirqs as soon as possible, but they
- * should not be able to lock up the box.
- */
-#define MAX_SOFTIRQ_TIME  msecs_to_jiffies(2)
-#define MAX_SOFTIRQ_RESTART 10
-
 #ifdef CONFIG_TRACE_IRQFLAGS
 /*
  * When we run softirqs from irq_exit() and thus on the hardirq stack we need
@@ -241,12 +225,10 @@ static inline void lockdep_softirq_end(bool in_hardirq) { }
 
 asmlinkage __visible void __softirq_entry __do_softirq(void)
 {
-	unsigned long end = jiffies + MAX_SOFTIRQ_TIME;
 	unsigned long old_flags = current->flags;
-	int max_restart = MAX_SOFTIRQ_RESTART;
 	struct softirq_action *h;
 	bool in_hardirq;
-	__u32 pending;
+	__u32 pending, executed = 0;
 	int softirq_bit;
 
 	/*
@@ -284,6 +266,9 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 		trace_softirq_entry(vec_nr);
 		h->action(h);
 		trace_softirq_exit(vec_nr);
+
+		executed |= 1 << vec_nr;
+
 		if (unlikely(prev_count != preempt_count())) {
 			pr_err("huh, entered softirq %u %s %p with preempt_count %08x, exited with %08x?\n",
 			       vec_nr, softirq_to_name[vec_nr], h->action,
@@ -299,11 +284,10 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 
 	pending = local_softirq_pending();
 	if (pending) {
-		if (time_before(jiffies, end) && !need_resched() &&
-		    --max_restart)
+		if (pending & executed || need_resched())
+			wakeup_softirqd();
+		else
 			goto restart;
-
-		wakeup_softirqd();
 	}
 
 	lockdep_softirq_end(in_hardirq);
-- 
2.7.4
