2020-09-15 12:17:18

by jun qian

[permalink] [raw]
Subject: [PATCH V7 0/4] Softirq:avoid large sched delay from the pending softirqs

From: jun qian <[email protected]>

When get the pending softirqs, it need to process all the pending
softirqs in the while loop. If the processing time of each pending
softirq is need more than 2 msec in this loop, or one of the softirq
will running a long time, according to the original code logic, it
will process all the pending softirqs without wakeuping ksoftirqd,
which will cause a relatively large scheduling delay on the
corresponding CPU, which we do not wish to see. The patch will check
the total time to process pending softirq, if the time exceeds 2 ms
we need to wakeup the ksofirqd to aviod large sched delay.

jun qian (4):
softirq: Use sched_clock() based timeout
softirq: Factor loop termination condition
softirq: Rewrite softirq processing loop
softirq: Allow early break the softirq processing loop

kernel/softirq.c | 168 +++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 120 insertions(+), 48 deletions(-)

--
1.8.3.1


2020-09-15 12:21:41

by jun qian

[permalink] [raw]
Subject: [PATCH V7 1/4] softirq: Use sched_clock() based timeout

From: jun qian <[email protected]>

Replace the jiffies based timeout with a sched_clock() based one.

Signed-off-by: jun qian <[email protected]>
---
kernel/softirq.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index c4201b7f..0db77ab 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -25,6 +25,7 @@
#include <linux/smpboot.h>
#include <linux/tick.h>
#include <linux/irq.h>
+#include <linux/sched/clock.h>

#define CREATE_TRACE_POINTS
#include <trace/events/irq.h>
@@ -210,7 +211,7 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
* 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_TIME (2 * NSEC_PER_MSEC)
#define MAX_SOFTIRQ_RESTART 10

#ifdef CONFIG_TRACE_IRQFLAGS
@@ -248,7 +249,7 @@ static inline void lockdep_softirq_end(bool in_hardirq) { }

asmlinkage __visible void __softirq_entry __do_softirq(void)
{
- unsigned long end = jiffies + MAX_SOFTIRQ_TIME;
+ u64 start = sched_clock();
unsigned long old_flags = current->flags;
int max_restart = MAX_SOFTIRQ_RESTART;
struct softirq_action *h;
@@ -307,7 +308,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)

pending = local_softirq_pending();
if (pending) {
- if (time_before(jiffies, end) && !need_resched() &&
+ if (sched_clock() - start < MAX_SOFTIRQ_TIME && !need_resched() &&
--max_restart)
goto restart;

--
1.8.3.1

2020-09-15 12:22:22

by jun qian

[permalink] [raw]
Subject: [PATCH V7 3/4] softirq: Rewrite softirq processing loop

From: jun qian <[email protected]>

Simplify the softirq processing loop by using the bitmap APIs

Signed-off-by: jun qian <[email protected]>
---
kernel/softirq.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index 5a627cd..cbb59b5 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -260,9 +260,9 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
unsigned long old_flags = current->flags;
unsigned int max_restart = MAX_SOFTIRQ_RESTART;
struct softirq_action *h;
+ unsigned long pending;
+ unsigned int vec_nr;
bool in_hardirq;
- __u32 pending;
- int softirq_bit;

/*
* Mask out PF_MEMALLOC as the current task context is borrowed for the
@@ -283,15 +283,13 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)

local_irq_enable();

- h = softirq_vec;
-
- while ((softirq_bit = ffs(pending))) {
- unsigned int vec_nr;
+ for_each_set_bit(vec_nr, &pending, NR_SOFTIRQS) {
int prev_count;

- h += softirq_bit - 1;
+ __clear_bit(vec_nr, &pending);
+
+ h = softirq_vec + vec_nr;

- vec_nr = h - softirq_vec;
prev_count = preempt_count();

kstat_incr_softirqs_this_cpu(vec_nr);
@@ -305,8 +303,6 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
prev_count, preempt_count());
preempt_count_set(prev_count);
}
- h++;
- pending >>= softirq_bit;
}

if (__this_cpu_read(ksoftirqd) == current)
--
1.8.3.1

2020-09-24 08:35:52

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH V7 1/4] softirq: Use sched_clock() based timeout

On Tue, Sep 15 2020 at 19:56, qianjun kernel wrote:
> From: jun qian <[email protected]>
>
> Replace the jiffies based timeout with a sched_clock() based one.

Changelogs have to explain the WHY not the WHAT. What the patch is doing
is already in te subject line and the exact WHAT (implementation) is in
the patch itself.

This applies to all 4 patches in this series. Each wants to have an
explanation of why this makes sense and which problem this is trying to solve.

Thanks,

tglx