lkml.org 
[lkml]   [2018]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: Fix 80d20d35af1e ("nohz: Fix local_timer_softirq_pending()") may have revealed another problem
On Fri, Dec 28, 2018 at 12:11:12AM +0100, Heiner Kallweit wrote:
>
> OK, did as you advised and here comes the trace. That's the related dmesg part:
>
> [ 1479.025092] x86: Booting SMP configuration:
> [ 1479.025129] smpboot: Booting Node 0 Processor 1 APIC 0x2
> [ 1479.094715] NOHZ: local_softirq_pending 202
> [ 1479.096557] smpboot: CPU 1 is now offline
>
> Hope it helps.
> Heiner
>
>
> # tracer: nop
> #
> # _-----=> irqs-off
> # / _----=> need-resched
> # | / _---=> hardirq/softirq
> # || / _--=> preempt-depth
> # ||| / delay
> # TASK-PID CPU# |||| TIMESTAMP FUNCTION
> # | | | |||| | |
[...]
> <idle>-0 [001] d.h2 1479.111017: softirq_raise: vec=9 [action=RCU]
> <idle>-0 [001] d.h2 1479.111026: softirq_raise: vec=7 [action=SCHED]
> <idle>-0 [001] ..s2 1479.111035: softirq_entry: vec=1 [action=TIMER]
> <idle>-0 [001] ..s2 1479.111040: softirq_exit: vec=1 [action=TIMER]
> <idle>-0 [001] ..s2 1479.111040: softirq_entry: vec=7 [action=SCHED]
> <idle>-0 [001] ..s2 1479.111052: softirq_exit: vec=7 [action=SCHED]
> <idle>-0 [001] ..s2 1479.111052: softirq_entry: vec=9 [action=RCU]
> <idle>-0 [001] .Ns2 1479.111079: softirq_exit: vec=9 [action=RCU]
> cpuhp/1-13 [001] dNh2 1479.112930: softirq_raise: vec=1 [action=TIMER]
> cpuhp/1-13 [001] dNh2 1479.112935: softirq_raise: vec=9 [action=RCU]

Interesting, the softirq is raised from hardirq but it's not handled in the end of
the IRQ. Are you running threaded IRQS by any chance? If so I would expect ksoftirqd
to handle the pending work before we go idle. However I can imagine a small window
where such an expectation may not be met: if the softirq is raised after the ksoftirqd
thread is parked (CPUHP_AP_SMPBOOT_THREADS), which is right before we disable the CPU
(CPUHP_TEARDOWN_CPU).

I don't know if we can afford to ignore a softirq even at this late stage. We should
probably avoid leaking any. So here is a possible fix, if you don't mind trying:

diff --git a/kernel/softirq.c b/kernel/softirq.c
index d288133..716096b 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -56,6 +56,7 @@ EXPORT_PER_CPU_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(int, ksoftirqd_parked);

const char * const softirq_to_name[NR_SOFTIRQS] = {
"HI", "TIMER", "NET_TX", "NET_RX", "BLOCK", "IRQ_POLL",
@@ -363,7 +364,7 @@ static inline void invoke_softirq(void)
if (ksoftirqd_running(local_softirq_pending()))
return;

- if (!force_irqthreads) {
+ if (!force_irqthreads || __this_cpu_read(ksoftirqd_parked)) {
#ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK
/*
* We can safely execute softirq on the current stack if
@@ -659,6 +660,22 @@ static void run_ksoftirqd(unsigned int cpu)
local_irq_enable();
}

+static void ksoftirqd_park(unsigned int cpu)
+{
+ local_irq_disable();
+ __this_cpu_write(ksoftirqd_parked, 1);
+
+ if (local_softirq_pending())
+ run_ksoftirqd(cpu);
+
+ local_irq_enable();
+}
+
+static void ksoftirqd_unpark(unsigned int cpu)
+{
+ __this_cpu_write(ksoftirqd_parked, 0);
+}
+
#ifdef CONFIG_HOTPLUG_CPU
/*
* tasklet_kill_immediate is called to remove a tasklet which can already be
@@ -724,6 +741,8 @@ static int takeover_tasklets(unsigned int cpu)
static struct smp_hotplug_thread softirq_threads = {
.store = &ksoftirqd,
.thread_should_run = ksoftirqd_should_run,
+ .park = ksoftirqd_park,
+ .unpark = ksoftirqd_unpark,
.thread_fn = run_ksoftirqd,
.thread_comm = "ksoftirqd/%u",
};
\
 
 \ /
  Last update: 2018-12-28 02:32    [W:0.125 / U:5.544 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site