* [patch] rt,sched,numa: Move task_numa_free() to __put_task_struct(), which -rt offloads
@ 2014-02-28 6:23 Mike Galbraith
2014-02-28 9:00 ` Pavel Vasilyev
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Mike Galbraith @ 2014-02-28 6:23 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Mel Gorman, LKML, RT
Bad idea:
[ 908.026136] [<ffffffff8150ad6a>] rt_spin_lock_slowlock+0xaa/0x2c0
[ 908.026145] [<ffffffff8108f701>] task_numa_free+0x31/0x130
[ 908.026151] [<ffffffff8108121e>] finish_task_switch+0xce/0x100
[ 908.026156] [<ffffffff81509c0a>] thread_return+0x48/0x4ae
[ 908.026160] [<ffffffff8150a095>] schedule+0x25/0xa0
[ 908.026163] [<ffffffff8150ad95>] rt_spin_lock_slowlock+0xd5/0x2c0
[ 908.026170] [<ffffffff810658cf>] get_signal_to_deliver+0xaf/0x680
[ 908.026175] [<ffffffff8100242d>] do_signal+0x3d/0x5b0
[ 908.026179] [<ffffffff81002a30>] do_notify_resume+0x90/0xe0
[ 908.026186] [<ffffffff81513176>] int_signal+0x12/0x17
[ 908.026193] [<00007ff2a388b1d0>] 0x7ff2a388b1cf
Signed-off-by: Mike Galbraith <bitbucket@online.de>
diff --git a/kernel/fork.c b/kernel/fork.c
index a17621c6cd42..332688e5e7b4 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -237,6 +237,7 @@ void __put_task_struct(struct task_struct *tsk)
WARN_ON(atomic_read(&tsk->usage));
WARN_ON(tsk == current);
+ task_numa_free(tsk);
security_task_free(tsk);
exit_creds(tsk);
delayacct_tsk_free(tsk);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 6edbef296ece..94a963fd040e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2149,8 +2149,6 @@ static void finish_task_switch(struct rq *rq, struct task_struct *prev)
if (mm)
mmdrop(mm);
if (unlikely(prev_state == TASK_DEAD)) {
- task_numa_free(prev);
-
if (prev->sched_class->task_dead)
prev->sched_class->task_dead(prev);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [patch] rt,sched,numa: Move task_numa_free() to __put_task_struct(), which -rt offloads
2014-02-28 6:23 [patch] rt,sched,numa: Move task_numa_free() to __put_task_struct(), which -rt offloads Mike Galbraith
@ 2014-02-28 9:00 ` Pavel Vasilyev
2014-02-28 11:32 ` Peter Zijlstra
2014-03-11 12:40 ` [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct() tip-bot for Mike Galbraith
2 siblings, 0 replies; 12+ messages in thread
From: Pavel Vasilyev @ 2014-02-28 9:00 UTC (permalink / raw)
Cc: RT
28.02.2014 10:23, Mike Galbraith пишет:
Maybe
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -237,6 +237,7 @@ void __put_task_struct(struct task_struct *tsk)
WARN_ON(atomic_read(&tsk->usage));
WARN_ON(tsk == current);
+#ifdef CONFIG_NUMA
+ task_numa_free(tsk);
+#endif
security_task_free(tsk);
exit_creds(tsk);
delayacct_tsk_free(tsk);
...
...
...
>
--
Pavel.
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] rt,sched,numa: Move task_numa_free() to __put_task_struct(), which -rt offloads
2014-02-28 6:23 [patch] rt,sched,numa: Move task_numa_free() to __put_task_struct(), which -rt offloads Mike Galbraith
2014-02-28 9:00 ` Pavel Vasilyev
@ 2014-02-28 11:32 ` Peter Zijlstra
2014-03-11 12:40 ` [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct() tip-bot for Mike Galbraith
2 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2014-02-28 11:32 UTC (permalink / raw)
To: Mike Galbraith; +Cc: Mel Gorman, LKML, RT
On Fri, Feb 28, 2014 at 07:23:11AM +0100, Mike Galbraith wrote:
>
> Bad idea:
> [ 908.026136] [<ffffffff8150ad6a>] rt_spin_lock_slowlock+0xaa/0x2c0
> [ 908.026145] [<ffffffff8108f701>] task_numa_free+0x31/0x130
> [ 908.026151] [<ffffffff8108121e>] finish_task_switch+0xce/0x100
> [ 908.026156] [<ffffffff81509c0a>] thread_return+0x48/0x4ae
> [ 908.026160] [<ffffffff8150a095>] schedule+0x25/0xa0
> [ 908.026163] [<ffffffff8150ad95>] rt_spin_lock_slowlock+0xd5/0x2c0
> [ 908.026170] [<ffffffff810658cf>] get_signal_to_deliver+0xaf/0x680
> [ 908.026175] [<ffffffff8100242d>] do_signal+0x3d/0x5b0
> [ 908.026179] [<ffffffff81002a30>] do_notify_resume+0x90/0xe0
> [ 908.026186] [<ffffffff81513176>] int_signal+0x12/0x17
> [ 908.026193] [<00007ff2a388b1d0>] 0x7ff2a388b1cf
>
> Signed-off-by: Mike Galbraith <bitbucket@online.de>
Fair enough; thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct()
2014-02-28 6:23 [patch] rt,sched,numa: Move task_numa_free() to __put_task_struct(), which -rt offloads Mike Galbraith
2014-02-28 9:00 ` Pavel Vasilyev
2014-02-28 11:32 ` Peter Zijlstra
@ 2014-03-11 12:40 ` tip-bot for Mike Galbraith
2014-04-06 19:17 ` Sasha Levin
2 siblings, 1 reply; 12+ messages in thread
From: tip-bot for Mike Galbraith @ 2014-03-11 12:40 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, torvalds, peterz, bitbucket, mgorman,
akpm, tglx
Commit-ID: 156654f491dd8d52687a5fbe1637f472a52ce75b
Gitweb: http://git.kernel.org/tip/156654f491dd8d52687a5fbe1637f472a52ce75b
Author: Mike Galbraith <bitbucket@online.de>
AuthorDate: Fri, 28 Feb 2014 07:23:11 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 11 Mar 2014 12:05:43 +0100
sched/numa: Move task_numa_free() to __put_task_struct()
Bad idea on -rt:
[ 908.026136] [<ffffffff8150ad6a>] rt_spin_lock_slowlock+0xaa/0x2c0
[ 908.026145] [<ffffffff8108f701>] task_numa_free+0x31/0x130
[ 908.026151] [<ffffffff8108121e>] finish_task_switch+0xce/0x100
[ 908.026156] [<ffffffff81509c0a>] thread_return+0x48/0x4ae
[ 908.026160] [<ffffffff8150a095>] schedule+0x25/0xa0
[ 908.026163] [<ffffffff8150ad95>] rt_spin_lock_slowlock+0xd5/0x2c0
[ 908.026170] [<ffffffff810658cf>] get_signal_to_deliver+0xaf/0x680
[ 908.026175] [<ffffffff8100242d>] do_signal+0x3d/0x5b0
[ 908.026179] [<ffffffff81002a30>] do_notify_resume+0x90/0xe0
[ 908.026186] [<ffffffff81513176>] int_signal+0x12/0x17
[ 908.026193] [<00007ff2a388b1d0>] 0x7ff2a388b1cf
and since upstream does not mind where we do this, be a bit nicer ...
Signed-off-by: Mike Galbraith <bitbucket@online.de>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Cc: Mel Gorman <mgorman@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1393568591.6018.27.camel@marge.simpson.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/fork.c | 1 +
kernel/sched/core.c | 2 --
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index a17621c..332688e 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -237,6 +237,7 @@ void __put_task_struct(struct task_struct *tsk)
WARN_ON(atomic_read(&tsk->usage));
WARN_ON(tsk == current);
+ task_numa_free(tsk);
security_task_free(tsk);
exit_creds(tsk);
delayacct_tsk_free(tsk);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index dd89c27..9e126a2 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2151,8 +2151,6 @@ static void finish_task_switch(struct rq *rq, struct task_struct *prev)
if (mm)
mmdrop(mm);
if (unlikely(prev_state == TASK_DEAD)) {
- task_numa_free(prev);
-
if (prev->sched_class->task_dead)
prev->sched_class->task_dead(prev);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct()
2014-03-11 12:40 ` [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct() tip-bot for Mike Galbraith
@ 2014-04-06 19:17 ` Sasha Levin
2014-04-07 5:29 ` Mike Galbraith
0 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2014-04-06 19:17 UTC (permalink / raw)
To: mingo, hpa, linux-kernel, torvalds, peterz, bitbucket, mgorman,
akpm, tglx, linux-tip-commits
Cc: Dave Jones
On 03/11/2014 08:40 AM, tip-bot for Mike Galbraith wrote:
> Commit-ID: 156654f491dd8d52687a5fbe1637f472a52ce75b
> Gitweb: http://git.kernel.org/tip/156654f491dd8d52687a5fbe1637f472a52ce75b
> Author: Mike Galbraith <bitbucket@online.de>
> AuthorDate: Fri, 28 Feb 2014 07:23:11 +0100
> Committer: Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 11 Mar 2014 12:05:43 +0100
>
> sched/numa: Move task_numa_free() to __put_task_struct()
>
> Bad idea on -rt:
>
> [ 908.026136] [<ffffffff8150ad6a>] rt_spin_lock_slowlock+0xaa/0x2c0
> [ 908.026145] [<ffffffff8108f701>] task_numa_free+0x31/0x130
> [ 908.026151] [<ffffffff8108121e>] finish_task_switch+0xce/0x100
> [ 908.026156] [<ffffffff81509c0a>] thread_return+0x48/0x4ae
> [ 908.026160] [<ffffffff8150a095>] schedule+0x25/0xa0
> [ 908.026163] [<ffffffff8150ad95>] rt_spin_lock_slowlock+0xd5/0x2c0
> [ 908.026170] [<ffffffff810658cf>] get_signal_to_deliver+0xaf/0x680
> [ 908.026175] [<ffffffff8100242d>] do_signal+0x3d/0x5b0
> [ 908.026179] [<ffffffff81002a30>] do_notify_resume+0x90/0xe0
> [ 908.026186] [<ffffffff81513176>] int_signal+0x12/0x17
> [ 908.026193] [<00007ff2a388b1d0>] 0x7ff2a388b1cf
>
> and since upstream does not mind where we do this, be a bit nicer ...
>
> Signed-off-by: Mike Galbraith <bitbucket@online.de>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> Cc: Mel Gorman <mgorman@suse.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Link: http://lkml.kernel.org/r/1393568591.6018.27.camel@marge.simpson.net
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
As it seems, upstream does mind:
[ 2590.260734] ======================================================
[ 2590.261695] [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ]
[ 2590.262748] 3.14.0-next-20140403-sasha-00022-g10224c0 #377 Tainted: G W
[ 2590.263846] ------------------------------------------------------
[ 2590.264730] trinity-c244/1210 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[ 2590.265783] (&(&grp->lock)->rlock){+.+...}, at: task_numa_free (kernel/sched/fair.c:1714)
[ 2590.267179]
[ 2590.267179] and this task is already holding:
[ 2590.267996] (&(&new_timer->it_lock)->rlock){-.....}, at: exit_itimers (kernel/posix-timers.c:971 kernel/posix-timers.c:998)
[ 2590.269381] which would create a new lock dependency:
[ 2590.270067] (&(&new_timer->it_lock)->rlock){-.....} -> (&(&grp->lock)->rlock){+.+...}
[ 2590.270067]
[ 2590.270067] but this new dependency connects a HARDIRQ-irq-safe lock:
[ 2590.270067] (&(&new_timer->it_lock)->rlock){-.....}
... which became HARDIRQ-irq-safe at:
[ 2590.270067] __lock_acquire (kernel/locking/lockdep.c:2783 kernel/locking/lockdep.c:3138)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock_irqsave (include/linux/spinlock_api_smp.h:117 kernel/locking/spinlock.c:159)
[ 2590.270067] posix_timer_fn (kernel/posix-timers.c:437)
[ 2590.270067] __run_hrtimer (kernel/hrtimer.c:1245 (discriminator 2))
[ 2590.270067] hrtimer_interrupt (kernel/hrtimer.c:1892)
[ 2590.270067] local_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:921)
[ 2590.270067] smp_apic_timer_interrupt (arch/x86/include/asm/apic.h:696 arch/x86/kernel/apic/apic.c:945)
[ 2590.270067] apic_timer_interrupt (arch/x86/kernel/entry_64.S:1164)
[ 2590.270067] default_idle (arch/x86/include/asm/paravirt.h:111 arch/x86/kernel/process.c:310)
[ 2590.270067] arch_cpu_idle (arch/x86/kernel/process.c:302)
[ 2590.270067] cpu_idle_loop (kernel/sched/idle.c:179 kernel/sched/idle.c:226)
[ 2590.270067] cpu_startup_entry (??:?)
[ 2590.270067] start_secondary (arch/x86/kernel/smpboot.c:267)
[ 2590.270067]
[ 2590.270067] to a HARDIRQ-irq-unsafe lock:
[ 2590.270067] (&(&grp->lock)->rlock){+.+...}
... which became HARDIRQ-irq-unsafe at:
[ 2590.270067] ... __lock_acquire (kernel/locking/lockdep.c:2800 kernel/locking/lockdep.c:3138)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock (include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:151)
[ 2590.270067] task_numa_placement (include/linux/nodemask.h:248 kernel/sched/fair.c:1504)
[ 2590.270067] task_numa_fault (kernel/sched/fair.c:1794)
[ 2590.270067] __handle_mm_fault (mm/memory.c:3796 mm/memory.c:3796 mm/memory.c:3909)
[ 2590.270067] handle_mm_fault (include/linux/memcontrol.h:148 mm/memory.c:3935)
[ 2590.270067] __do_page_fault (arch/x86/mm/fault.c:1220)
[ 2590.270067] do_page_fault (arch/x86/mm/fault.c:1272 include/linux/jump_label.h:105 include/linux/context_tracking_state.h:27 include/linux/context_tracking.h:45 arch/x86/mm/fault.c:1273)
[ 2590.270067] do_async_page_fault (arch/x86/kernel/kvm.c:263)
[ 2590.270067] async_page_fault (arch/x86/kernel/entry_64.S:1496)
[ 2590.270067]
[ 2590.270067] other info that might help us debug this:
[ 2590.270067]
[ 2590.270067] Possible interrupt unsafe locking scenario:
[ 2590.270067]
[ 2590.270067] CPU0 CPU1
[ 2590.270067] ---- ----
[ 2590.270067] lock(&(&grp->lock)->rlock);
[ 2590.270067] local_irq_disable();
[ 2590.270067] lock(&(&new_timer->it_lock)->rlock);
[ 2590.270067] lock(&(&grp->lock)->rlock);
[ 2590.270067] <Interrupt>
[ 2590.270067] lock(&(&new_timer->it_lock)->rlock);
[ 2590.270067]
[ 2590.270067] *** DEADLOCK ***
[ 2590.270067]
[ 2590.270067] 1 lock held by trinity-c244/1210:
[ 2590.270067] #0: (&(&new_timer->it_lock)->rlock){-.....}, at: exit_itimers (kernel/posix-timers.c:971 kernel/posix-timers.c:998)
[ 2590.270067]
the dependencies between HARDIRQ-irq-safe lock and the holding lock:
[ 2590.270067] -> (&(&new_timer->it_lock)->rlock){-.....} ops: 361 {
[ 2590.270067] IN-HARDIRQ-W at:
[ 2590.270067] __lock_acquire (kernel/locking/lockdep.c:2783 kernel/locking/lockdep.c:3138)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock_irqsave (include/linux/spinlock_api_smp.h:117 kernel/locking/spinlock.c:159)
[ 2590.270067] posix_timer_fn (kernel/posix-timers.c:437)
[ 2590.270067] __run_hrtimer (kernel/hrtimer.c:1245 (discriminator 2))
[ 2590.270067] hrtimer_interrupt (kernel/hrtimer.c:1892)
[ 2590.270067] local_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:921)
[ 2590.270067] smp_apic_timer_interrupt (arch/x86/include/asm/apic.h:696 arch/x86/kernel/apic/apic.c:945)
[ 2590.270067] apic_timer_interrupt (arch/x86/kernel/entry_64.S:1164)
[ 2590.270067] default_idle (arch/x86/include/asm/paravirt.h:111 arch/x86/kernel/process.c:310)
[ 2590.270067] arch_cpu_idle (arch/x86/kernel/process.c:302)
[ 2590.270067] cpu_idle_loop (kernel/sched/idle.c:179 kernel/sched/idle.c:226)
[ 2590.270067] cpu_startup_entry (??:?)
[ 2590.270067] start_secondary (arch/x86/kernel/smpboot.c:267)
[ 2590.270067] INITIAL USE at:
[ 2590.270067] __lock_acquire (kernel/locking/lockdep.c:3142)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock_irqsave (include/linux/spinlock_api_smp.h:117 kernel/locking/spinlock.c:159)
[ 2590.270067] exit_itimers (kernel/posix-timers.c:971 kernel/posix-timers.c:998)
[ 2590.270067] do_exit (kernel/exit.c:766)
[ 2590.270067] do_group_exit (kernel/exit.c:919)
[ 2590.270067] SyS_exit_group (kernel/exit.c:930)
[ 2590.270067] tracesys (arch/x86/kernel/entry_64.S:749)
[ 2590.270067] }
[ 2590.270067] ... key at: __key.33130 (??:?)
[ 2590.270067] ... acquired at:
[ 2590.270067] check_irq_usage (kernel/locking/lockdep.c:1638)
[ 2590.270067] __lock_acquire (kernel/locking/lockdep_states.h:7 kernel/locking/lockdep.c:1844 kernel/locking/lockdep.c:1945 kernel/locking/lockdep.c:2131 kernel/locking/lockdep.c:3182)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock (include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:151)
[ 2590.270067] task_numa_free (kernel/sched/fair.c:1714)
[ 2590.270067] __put_task_struct (kernel/fork.c:244 (discriminator 2))
[ 2590.270067] posix_cpu_timer_del (include/linux/sched.h:1807 kernel/posix-cpu-timers.c:409)
[ 2590.270067] exit_itimers (kernel/posix-timers.c:973 kernel/posix-timers.c:998)
[ 2590.270067] do_exit (kernel/exit.c:766)
[ 2590.270067] do_group_exit (kernel/exit.c:919)
[ 2590.270067] SyS_exit_group (kernel/exit.c:930)
[ 2590.270067] tracesys (arch/x86/kernel/entry_64.S:749)
[ 2590.270067]
[ 2590.270067]
the dependencies between the lock to be acquired and HARDIRQ-irq-unsafe lock:
[ 2590.270067] -> (&(&grp->lock)->rlock){+.+...} ops: 91 {
[ 2590.270067] HARDIRQ-ON-W at:
[ 2590.270067] __lock_acquire (kernel/locking/lockdep.c:2800 kernel/locking/lockdep.c:3138)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock (include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:151)
[ 2590.270067] task_numa_placement (include/linux/nodemask.h:248 kernel/sched/fair.c:1504)
[ 2590.270067] task_numa_fault (kernel/sched/fair.c:1794)
[ 2590.270067] __handle_mm_fault (mm/memory.c:3796 mm/memory.c:3796 mm/memory.c:3909)
[ 2590.270067] handle_mm_fault (include/linux/memcontrol.h:148 mm/memory.c:3935)
[ 2590.270067] __do_page_fault (arch/x86/mm/fault.c:1220)
[ 2590.270067] do_page_fault (arch/x86/mm/fault.c:1272 include/linux/jump_label.h:105 include/linux/context_tracking_state.h:27 include/linux/context_tracking.h:45 arch/x86/mm/fault.c:1273)
[ 2590.270067] do_async_page_fault (arch/x86/kernel/kvm.c:263)
[ 2590.270067] async_page_fault (arch/x86/kernel/entry_64.S:1496)
[ 2590.270067] SOFTIRQ-ON-W at:
[ 2590.270067] __lock_acquire (kernel/locking/lockdep.c:2804 kernel/locking/lockdep.c:3138)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock (include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:151)
[ 2590.270067] task_numa_placement (include/linux/nodemask.h:248 kernel/sched/fair.c:1504)
[ 2590.270067] task_numa_fault (kernel/sched/fair.c:1794)
[ 2590.270067] __handle_mm_fault (mm/memory.c:3796 mm/memory.c:3796 mm/memory.c:3909)
[ 2590.270067] handle_mm_fault (include/linux/memcontrol.h:148 mm/memory.c:3935)
[ 2590.270067] __do_page_fault (arch/x86/mm/fault.c:1220)
[ 2590.270067] do_page_fault (arch/x86/mm/fault.c:1272 include/linux/jump_label.h:105 include/linux/context_tracking_state.h:27 include/linux/context_tracking.h:45 arch/x86/mm/fault.c:1273)
[ 2590.270067] do_async_page_fault (arch/x86/kernel/kvm.c:263)
[ 2590.270067] async_page_fault (arch/x86/kernel/entry_64.S:1496)
[ 2590.270067] INITIAL USE at:
[ 2590.270067] __lock_acquire (kernel/locking/lockdep.c:3142)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock (include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:151)
[ 2590.270067] task_numa_placement (include/linux/nodemask.h:248 kernel/sched/fair.c:1504)
[ 2590.270067] task_numa_fault (kernel/sched/fair.c:1794)
[ 2590.270067] __handle_mm_fault (mm/memory.c:3796 mm/memory.c:3796 mm/memory.c:3909)
[ 2590.270067] handle_mm_fault (include/linux/memcontrol.h:148 mm/memory.c:3935)
[ 2590.270067] __do_page_fault (arch/x86/mm/fault.c:1220)
[ 2590.270067] do_page_fault (arch/x86/mm/fault.c:1272 include/linux/jump_label.h:105 include/linux/context_tracking_state.h:27 include/linux/context_tracking.h:45 arch/x86/mm/fault.c:1273)
[ 2590.270067] do_async_page_fault (arch/x86/kernel/kvm.c:263)
[ 2590.270067] async_page_fault (arch/x86/kernel/entry_64.S:1496)
[ 2590.270067] }
[ 2590.270067] ... key at: __key.32449 (??:?)
[ 2590.270067] ... acquired at:
[ 2590.270067] check_irq_usage (kernel/locking/lockdep.c:1638)
[ 2590.270067] __lock_acquire (kernel/locking/lockdep_states.h:7 kernel/locking/lockdep.c:1844 kernel/locking/lockdep.c:1945 kernel/locking/lockdep.c:2131 kernel/locking/lockdep.c:3182)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] _raw_spin_lock (include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:151)
[ 2590.270067] task_numa_free (kernel/sched/fair.c:1714)
[ 2590.270067] __put_task_struct (kernel/fork.c:244 (discriminator 2))
[ 2590.270067] posix_cpu_timer_del (include/linux/sched.h:1807 kernel/posix-cpu-timers.c:409)
[ 2590.270067] exit_itimers (kernel/posix-timers.c:973 kernel/posix-timers.c:998)
[ 2590.270067] do_exit (kernel/exit.c:766)
[ 2590.270067] do_group_exit (kernel/exit.c:919)
[ 2590.270067] SyS_exit_group (kernel/exit.c:930)
[ 2590.270067] tracesys (arch/x86/kernel/entry_64.S:749)
[ 2590.270067]
[ 2590.270067]
[ 2590.270067] stack backtrace:
[ 2590.270067] CPU: 3 PID: 1210 Comm: trinity-c244 Tainted: G W 3.14.0-next-20140403-sasha-00022-g10224c0 #377
[ 2590.270067] ffffffff87a83b60 ffff880081695ad8 ffffffff844bfb3f 0000000000000000
[ 2590.270067] ffff880081698cf0 ffff880081695be8 ffffffff811c0d05 0000000000000000
[ 2590.270067] ffffffff00000000 ffff880000000001 ffffffff8107aac5 ffff880081695b38
[ 2590.270067] Call Trace:
[ 2590.270067] dump_stack (lib/dump_stack.c:52)
[ 2590.270067] check_usage (kernel/locking/lockdep.c:1549 kernel/locking/lockdep.c:1580)
[ 2590.270067] ? sched_clock (arch/x86/include/asm/paravirt.h:192 arch/x86/kernel/tsc.c:305)
[ 2590.270067] ? sched_clock_cpu (kernel/sched/clock.c:311)
[ 2590.270067] check_irq_usage (kernel/locking/lockdep.c:1638)
[ 2590.270067] __lock_acquire (kernel/locking/lockdep_states.h:7 kernel/locking/lockdep.c:1844 kernel/locking/lockdep.c:1945 kernel/locking/lockdep.c:2131 kernel/locking/lockdep.c:3182)
[ 2590.270067] ? __lock_acquire (kernel/locking/lockdep.c:3189)
[ 2590.270067] lock_acquire (arch/x86/include/asm/current.h:14 kernel/locking/lockdep.c:3602)
[ 2590.270067] ? task_numa_free (kernel/sched/fair.c:1714)
[ 2590.270067] _raw_spin_lock (include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:151)
[ 2590.270067] ? task_numa_free (kernel/sched/fair.c:1714)
[ 2590.270067] ? trace_hardirqs_off_caller (kernel/locking/lockdep.c:2638 (discriminator 2))
[ 2590.270067] task_numa_free (kernel/sched/fair.c:1714)
[ 2590.270067] __put_task_struct (kernel/fork.c:244 (discriminator 2))
[ 2590.270067] posix_cpu_timer_del (include/linux/sched.h:1807 kernel/posix-cpu-timers.c:409)
[ 2590.270067] exit_itimers (kernel/posix-timers.c:973 kernel/posix-timers.c:998)
[ 2590.270067] do_exit (kernel/exit.c:766)
[ 2590.270067] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[ 2590.270067] ? trace_hardirqs_on_caller (kernel/locking/lockdep.c:2557 kernel/locking/lockdep.c:2599)
[ 2590.270067] ? trace_hardirqs_on (kernel/locking/lockdep.c:2607)
[ 2590.270067] do_group_exit (kernel/exit.c:919)
[ 2590.270067] SyS_exit_group (kernel/exit.c:930)
[ 2590.270067] tracesys (arch/x86/kernel/entry_64.S:749)
Thanks,
Sasha
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct()
2014-04-06 19:17 ` Sasha Levin
@ 2014-04-07 5:29 ` Mike Galbraith
2014-04-07 7:30 ` Mike Galbraith
0 siblings, 1 reply; 12+ messages in thread
From: Mike Galbraith @ 2014-04-07 5:29 UTC (permalink / raw)
To: Sasha Levin
Cc: mingo, hpa, linux-kernel, torvalds, peterz, mgorman, akpm, tglx,
linux-tip-commits, Dave Jones
On Sun, 2014-04-06 at 15:17 -0400, Sasha Levin wrote:
> On 03/11/2014 08:40 AM, tip-bot for Mike Galbraith wrote:
> > Commit-ID: 156654f491dd8d52687a5fbe1637f472a52ce75b
> > Gitweb: http://git.kernel.org/tip/156654f491dd8d52687a5fbe1637f472a52ce75b
> > Author: Mike Galbraith <bitbucket@online.de>
> > AuthorDate: Fri, 28 Feb 2014 07:23:11 +0100
> > Committer: Ingo Molnar <mingo@kernel.org>
> > CommitDate: Tue, 11 Mar 2014 12:05:43 +0100
> >
> > sched/numa: Move task_numa_free() to __put_task_struct()
> >
> > Bad idea on -rt:
> >
> > [ 908.026136] [<ffffffff8150ad6a>] rt_spin_lock_slowlock+0xaa/0x2c0
> > [ 908.026145] [<ffffffff8108f701>] task_numa_free+0x31/0x130
> > [ 908.026151] [<ffffffff8108121e>] finish_task_switch+0xce/0x100
> > [ 908.026156] [<ffffffff81509c0a>] thread_return+0x48/0x4ae
> > [ 908.026160] [<ffffffff8150a095>] schedule+0x25/0xa0
> > [ 908.026163] [<ffffffff8150ad95>] rt_spin_lock_slowlock+0xd5/0x2c0
> > [ 908.026170] [<ffffffff810658cf>] get_signal_to_deliver+0xaf/0x680
> > [ 908.026175] [<ffffffff8100242d>] do_signal+0x3d/0x5b0
> > [ 908.026179] [<ffffffff81002a30>] do_notify_resume+0x90/0xe0
> > [ 908.026186] [<ffffffff81513176>] int_signal+0x12/0x17
> > [ 908.026193] [<00007ff2a388b1d0>] 0x7ff2a388b1cf
> >
> > and since upstream does not mind where we do this, be a bit nicer ...
> >
> > Signed-off-by: Mike Galbraith <bitbucket@online.de>
> > Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> > Cc: Mel Gorman <mgorman@suse.com>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Link: http://lkml.kernel.org/r/1393568591.6018.27.camel@marge.simpson.net
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
>
> As it seems, upstream does mind:
>
> [ 2590.260734] ======================================================
> [ 2590.261695] [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ]
> [ 2590.262748] 3.14.0-next-20140403-sasha-00022-g10224c0 #377 Tainted: G W
> [ 2590.263846] ------------------------------------------------------
> [ 2590.264730] trinity-c244/1210 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> [ 2590.265783] (&(&grp->lock)->rlock){+.+...}, at: task_numa_free (kernel/sched/fair.c:1714)
> [ 2590.267179]
> [ 2590.267179] and this task is already holding:
> [ 2590.267996] (&(&new_timer->it_lock)->rlock){-.....}, at: exit_itimers (kernel/posix-timers.c:971 kernel/posix-timers.c:998)
> [ 2590.269381] which would create a new lock dependency:
> [ 2590.270067] (&(&new_timer->it_lock)->rlock){-.....} -> (&(&grp->lock)->rlock){+.+...}
I'm not getting it.
I moved task_numa_free() from one interrupts enabled spot to another.
But, with numa=fake=4 and lockdep enabled, not only does lockdep gripe,
my little box locks up on splat. Saying spin_lock/unlock_irq() did the
expected, just moved lockdep gripe to task_numa_fault().
> [ 2590.270067] Possible interrupt unsafe locking scenario:
> [ 2590.270067]
> [ 2590.270067] CPU0 CPU1
> [ 2590.270067] ---- ----
> [ 2590.270067] lock(&(&grp->lock)->rlock);
> [ 2590.270067] local_irq_disable();
> [ 2590.270067] lock(&(&new_timer->it_lock)->rlock);
> [ 2590.270067] lock(&(&grp->lock)->rlock);
> [ 2590.270067] <Interrupt>
> [ 2590.270067] lock(&(&new_timer->it_lock)->rlock);
> [ 2590.270067]
> [ 2590.270067] *** DEADLOCK ***
Ok, so how did I manage that HARDIRQ-safe -> HARDIRQ-unsafe?
-Mike
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct()
2014-04-07 5:29 ` Mike Galbraith
@ 2014-04-07 7:30 ` Mike Galbraith
2014-04-07 8:16 ` Peter Zijlstra
0 siblings, 1 reply; 12+ messages in thread
From: Mike Galbraith @ 2014-04-07 7:30 UTC (permalink / raw)
To: Sasha Levin
Cc: mingo, hpa, linux-kernel, torvalds, peterz, mgorman, akpm, tglx,
linux-tip-commits, Dave Jones
On Mon, 2014-04-07 at 07:29 +0200, Mike Galbraith wrote:
> I'm not getting it.
>
> I moved task_numa_free() from one interrupts enabled spot to another.
> But, with numa=fake=4 and lockdep enabled, not only does lockdep gripe,
> my little box locks up on splat. Saying spin_lock/unlock_irq() did the
> expected, just moved lockdep gripe to task_numa_fault().
>
>
> > [ 2590.270067] Possible interrupt unsafe locking scenario:
> > [ 2590.270067]
> > [ 2590.270067] CPU0 CPU1
> > [ 2590.270067] ---- ----
> > [ 2590.270067] lock(&(&grp->lock)->rlock);
> > [ 2590.270067] local_irq_disable();
> > [ 2590.270067] lock(&(&new_timer->it_lock)->rlock);
> > [ 2590.270067] lock(&(&grp->lock)->rlock);
> > [ 2590.270067] <Interrupt>
> > [ 2590.270067] lock(&(&new_timer->it_lock)->rlock);
> > [ 2590.270067]
> > [ 2590.270067] *** DEADLOCK ***
>
> Ok, so how did I manage that HARDIRQ-safe -> HARDIRQ-unsafe?
Think I'll turn lockdep off, and make context switches take a good long
while after finish_lock_switch(), but meanwhile, this made it happy.
Sasha reports that lockdep claims 156654f491dd8d52687a5fbe1637f472a52ce75b made
numa_group.lock interrupt unsafe. While I don't see how that could be given the
commit in question moved task_numa_free() from one irq enabled region to another,
the below does make both gripes and lockup upon gripe with numa=fake=4 go away.
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Not-signed-off-by: Mike Galbraith <bitbucket@online.de>
---
kernel/sched/fair.c | 12 +++++++-----
kernel/sched/sched.h | 9 +++++++++
2 files changed, 16 insertions(+), 5 deletions(-)
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1497,7 +1497,7 @@ static void task_numa_placement(struct t
/* If the task is part of a group prevent parallel updates to group stats */
if (p->numa_group) {
group_lock = &p->numa_group->lock;
- spin_lock(group_lock);
+ spin_lock_irq(group_lock);
}
/* Find the node with the highest number of faults */
@@ -1572,7 +1572,7 @@ static void task_numa_placement(struct t
}
}
- spin_unlock(group_lock);
+ spin_unlock_irq(group_lock);
}
/* Preferred node as the node with the most faults */
@@ -1677,7 +1677,8 @@ static void task_numa_group(struct task_
if (!join)
return;
- double_lock(&my_grp->lock, &grp->lock);
+ BUG_ON(irqs_disabled());
+ double_lock_irq(&my_grp->lock, &grp->lock);
for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++) {
my_grp->faults[i] -= p->numa_faults_memory[i];
@@ -1692,6 +1693,7 @@ static void task_numa_group(struct task_
spin_unlock(&my_grp->lock);
spin_unlock(&grp->lock);
+ local_irq_enable();
rcu_assign_pointer(p->numa_group, grp);
@@ -1710,14 +1712,14 @@ void task_numa_free(struct task_struct *
void *numa_faults = p->numa_faults_memory;
if (grp) {
- spin_lock(&grp->lock);
+ spin_lock_irq(&grp->lock);
for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++)
grp->faults[i] -= p->numa_faults_memory[i];
grp->total_faults -= p->total_numa_faults;
list_del(&p->numa_entry);
grp->nr_tasks--;
- spin_unlock(&grp->lock);
+ spin_unlock_irq(&grp->lock);
rcu_assign_pointer(p->numa_group, NULL);
put_numa_group(grp);
}
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1388,6 +1388,15 @@ static inline void double_lock(spinlock_
spin_lock_nested(l2, SINGLE_DEPTH_NESTING);
}
+static inline void double_lock_irq(spinlock_t *l1, spinlock_t *l2)
+{
+ if (l1 > l2)
+ swap(l1, l2);
+
+ spin_lock_irq(l1);
+ spin_lock_nested(l2, SINGLE_DEPTH_NESTING);
+}
+
static inline void double_raw_lock(raw_spinlock_t *l1, raw_spinlock_t *l2)
{
if (l1 > l2)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct()
2014-04-07 7:30 ` Mike Galbraith
@ 2014-04-07 8:16 ` Peter Zijlstra
2014-04-07 8:40 ` Mike Galbraith
2014-04-07 8:55 ` Mike Galbraith
0 siblings, 2 replies; 12+ messages in thread
From: Peter Zijlstra @ 2014-04-07 8:16 UTC (permalink / raw)
To: Mike Galbraith
Cc: Sasha Levin, mingo, hpa, linux-kernel, torvalds, mgorman, akpm,
tglx, linux-tip-commits, Dave Jones
On Mon, Apr 07, 2014 at 09:30:30AM +0200, Mike Galbraith wrote:
> - double_lock(&my_grp->lock, &grp->lock);
> + BUG_ON(irqs_disabled());
> + double_lock_irq(&my_grp->lock, &grp->lock);
So either make this:
local_irq_disable();
double_lock();
or
>
> for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++) {
> my_grp->faults[i] -= p->numa_faults_memory[i];
> @@ -1692,6 +1693,7 @@ static void task_numa_group(struct task_
>
> spin_unlock(&my_grp->lock);
> spin_unlock(&grp->lock);
> + local_irq_enable();
use:
spin_unlock()
spin_unlock_irq()
or so, but this imbalance is making my itch :-)
>
> rcu_assign_pointer(p->numa_group, grp);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct()
2014-04-07 8:16 ` Peter Zijlstra
@ 2014-04-07 8:40 ` Mike Galbraith
2014-04-07 8:55 ` Mike Galbraith
1 sibling, 0 replies; 12+ messages in thread
From: Mike Galbraith @ 2014-04-07 8:40 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sasha Levin, mingo, hpa, linux-kernel, torvalds, mgorman, akpm,
tglx, linux-tip-commits, Dave Jones
On Mon, 2014-04-07 at 10:16 +0200, Peter Zijlstra wrote:
> On Mon, Apr 07, 2014 at 09:30:30AM +0200, Mike Galbraith wrote:
> > - double_lock(&my_grp->lock, &grp->lock);
> > + BUG_ON(irqs_disabled());
> > + double_lock_irq(&my_grp->lock, &grp->lock);
>
> So either make this:
>
> local_irq_disable();
> double_lock();
>
> or
>
> >
> > for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++) {
> > my_grp->faults[i] -= p->numa_faults_memory[i];
> > @@ -1692,6 +1693,7 @@ static void task_numa_group(struct task_
> >
> > spin_unlock(&my_grp->lock);
> > spin_unlock(&grp->lock);
> > + local_irq_enable();
>
> use:
> spin_unlock()
> spin_unlock_irq()
*thwap* Well duh.
> or so, but this imbalance is making my itch :-)
Yeah, much better.
Before I actually sign that off, mind cluing me in as to why I should
not be sitting here thinking lockdep smoked its breakfast?
-Mike
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct()
2014-04-07 8:16 ` Peter Zijlstra
2014-04-07 8:40 ` Mike Galbraith
@ 2014-04-07 8:55 ` Mike Galbraith
2014-04-13 20:53 ` Govindarajulu Varadarajan
2014-04-14 7:22 ` [tip:sched/urgent] sched/numa: Fix task_numa_free() lockdep splat tip-bot for Mike Galbraith
1 sibling, 2 replies; 12+ messages in thread
From: Mike Galbraith @ 2014-04-07 8:55 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sasha Levin, mingo, hpa, linux-kernel, torvalds, mgorman, akpm,
tglx, linux-tip-commits, Dave Jones
On Mon, 2014-04-07 at 10:16 +0200, Peter Zijlstra wrote:
> On Mon, Apr 07, 2014 at 09:30:30AM +0200, Mike Galbraith wrote:
> > - double_lock(&my_grp->lock, &grp->lock);
> > + BUG_ON(irqs_disabled());
> > + double_lock_irq(&my_grp->lock, &grp->lock);
>
> So either make this:
>
> local_irq_disable();
> double_lock();
>
> or
>
> >
> > for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++) {
> > my_grp->faults[i] -= p->numa_faults_memory[i];
> > @@ -1692,6 +1693,7 @@ static void task_numa_group(struct task_
> >
> > spin_unlock(&my_grp->lock);
> > spin_unlock(&grp->lock);
> > + local_irq_enable();
>
> use:
> spin_unlock()
> spin_unlock_irq()
>
> or so, but this imbalance is making my itch :-)
sched, numa: fix task_numa_free() lockdep splat
Sasha reports that lockdep claims 156654f491dd8d52687a5fbe1637f472a52ce75b made
numa_group.lock interrupt unsafe. While I don't see how that could be given the
commit in question moved task_numa_free() from one irq enabled region to another,
the below does make both gripes and lockups upon gripe with numa=fake=4 go away.
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Mike Galbraith <bitbucket@online.de>
---
kernel/sched/fair.c | 13 +++++++------
kernel/sched/sched.h | 9 +++++++++
2 files changed, 16 insertions(+), 6 deletions(-)
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1497,7 +1497,7 @@ static void task_numa_placement(struct t
/* If the task is part of a group prevent parallel updates to group stats */
if (p->numa_group) {
group_lock = &p->numa_group->lock;
- spin_lock(group_lock);
+ spin_lock_irq(group_lock);
}
/* Find the node with the highest number of faults */
@@ -1572,7 +1572,7 @@ static void task_numa_placement(struct t
}
}
- spin_unlock(group_lock);
+ spin_unlock_irq(group_lock);
}
/* Preferred node as the node with the most faults */
@@ -1677,7 +1677,8 @@ static void task_numa_group(struct task_
if (!join)
return;
- double_lock(&my_grp->lock, &grp->lock);
+ BUG_ON(irqs_disabled());
+ double_lock_irq(&my_grp->lock, &grp->lock);
for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++) {
my_grp->faults[i] -= p->numa_faults_memory[i];
@@ -1691,7 +1692,7 @@ static void task_numa_group(struct task_
grp->nr_tasks++;
spin_unlock(&my_grp->lock);
- spin_unlock(&grp->lock);
+ spin_unlock_irq(&grp->lock);
rcu_assign_pointer(p->numa_group, grp);
@@ -1710,14 +1711,14 @@ void task_numa_free(struct task_struct *
void *numa_faults = p->numa_faults_memory;
if (grp) {
- spin_lock(&grp->lock);
+ spin_lock_irq(&grp->lock);
for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++)
grp->faults[i] -= p->numa_faults_memory[i];
grp->total_faults -= p->total_numa_faults;
list_del(&p->numa_entry);
grp->nr_tasks--;
- spin_unlock(&grp->lock);
+ spin_unlock_irq(&grp->lock);
rcu_assign_pointer(p->numa_group, NULL);
put_numa_group(grp);
}
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1388,6 +1388,15 @@ static inline void double_lock(spinlock_
spin_lock_nested(l2, SINGLE_DEPTH_NESTING);
}
+static inline void double_lock_irq(spinlock_t *l1, spinlock_t *l2)
+{
+ if (l1 > l2)
+ swap(l1, l2);
+
+ spin_lock_irq(l1);
+ spin_lock_nested(l2, SINGLE_DEPTH_NESTING);
+}
+
static inline void double_raw_lock(raw_spinlock_t *l1, raw_spinlock_t *l2)
{
if (l1 > l2)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct()
2014-04-07 8:55 ` Mike Galbraith
@ 2014-04-13 20:53 ` Govindarajulu Varadarajan
2014-04-14 7:22 ` [tip:sched/urgent] sched/numa: Fix task_numa_free() lockdep splat tip-bot for Mike Galbraith
1 sibling, 0 replies; 12+ messages in thread
From: Govindarajulu Varadarajan @ 2014-04-13 20:53 UTC (permalink / raw)
To: Mike Galbraith
Cc: Peter Zijlstra, Sasha Levin, mingo, hpa, linux-kernel, torvalds,
mgorman, akpm, tglx, linux-tip-commits, Dave Jones
On Mon, 7 Apr 2014, Mike Galbraith wrote:
> On Mon, 2014-04-07 at 10:16 +0200, Peter Zijlstra wrote:
>> On Mon, Apr 07, 2014 at 09:30:30AM +0200, Mike Galbraith wrote:
>>> - double_lock(&my_grp->lock, &grp->lock);
>>> + BUG_ON(irqs_disabled());
>>> + double_lock_irq(&my_grp->lock, &grp->lock);
>>
>> So either make this:
>>
>> local_irq_disable();
>> double_lock();
>>
>> or
>>
>>>
>>> for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++) {
>>> my_grp->faults[i] -= p->numa_faults_memory[i];
>>> @@ -1692,6 +1693,7 @@ static void task_numa_group(struct task_
>>>
>>> spin_unlock(&my_grp->lock);
>>> spin_unlock(&grp->lock);
>>> + local_irq_enable();
>>
>> use:
>> spin_unlock()
>> spin_unlock_irq()
>>
>> or so, but this imbalance is making my itch :-)
>
> sched, numa: fix task_numa_free() lockdep splat
>
> Sasha reports that lockdep claims 156654f491dd8d52687a5fbe1637f472a52ce75b made
> numa_group.lock interrupt unsafe. While I don't see how that could be given the
> commit in question moved task_numa_free() from one irq enabled region to another,
> the below does make both gripes and lockups upon gripe with numa=fake=4 go away.
>
Hi
I Am hitting this bug quite frequently. I do not see the problem after applying
this patch.
Thanks
Tested-by: Govindarajulu Varadarajan <govindx7c6@gmail.com>
> Reported-by: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Mike Galbraith <bitbucket@online.de>
> ---
> kernel/sched/fair.c | 13 +++++++------
> kernel/sched/sched.h | 9 +++++++++
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1497,7 +1497,7 @@ static void task_numa_placement(struct t
> /* If the task is part of a group prevent parallel updates to group stats */
> if (p->numa_group) {
> group_lock = &p->numa_group->lock;
> - spin_lock(group_lock);
> + spin_lock_irq(group_lock);
> }
>
> /* Find the node with the highest number of faults */
> @@ -1572,7 +1572,7 @@ static void task_numa_placement(struct t
> }
> }
>
> - spin_unlock(group_lock);
> + spin_unlock_irq(group_lock);
> }
>
> /* Preferred node as the node with the most faults */
> @@ -1677,7 +1677,8 @@ static void task_numa_group(struct task_
> if (!join)
> return;
>
> - double_lock(&my_grp->lock, &grp->lock);
> + BUG_ON(irqs_disabled());
> + double_lock_irq(&my_grp->lock, &grp->lock);
>
> for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++) {
> my_grp->faults[i] -= p->numa_faults_memory[i];
> @@ -1691,7 +1692,7 @@ static void task_numa_group(struct task_
> grp->nr_tasks++;
>
> spin_unlock(&my_grp->lock);
> - spin_unlock(&grp->lock);
> + spin_unlock_irq(&grp->lock);
>
> rcu_assign_pointer(p->numa_group, grp);
>
> @@ -1710,14 +1711,14 @@ void task_numa_free(struct task_struct *
> void *numa_faults = p->numa_faults_memory;
>
> if (grp) {
> - spin_lock(&grp->lock);
> + spin_lock_irq(&grp->lock);
> for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++)
> grp->faults[i] -= p->numa_faults_memory[i];
> grp->total_faults -= p->total_numa_faults;
>
> list_del(&p->numa_entry);
> grp->nr_tasks--;
> - spin_unlock(&grp->lock);
> + spin_unlock_irq(&grp->lock);
> rcu_assign_pointer(p->numa_group, NULL);
> put_numa_group(grp);
> }
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1388,6 +1388,15 @@ static inline void double_lock(spinlock_
> spin_lock_nested(l2, SINGLE_DEPTH_NESTING);
> }
>
> +static inline void double_lock_irq(spinlock_t *l1, spinlock_t *l2)
> +{
> + if (l1 > l2)
> + swap(l1, l2);
> +
> + spin_lock_irq(l1);
> + spin_lock_nested(l2, SINGLE_DEPTH_NESTING);
> +}
> +
> static inline void double_raw_lock(raw_spinlock_t *l1, raw_spinlock_t *l2)
> {
> if (l1 > l2)
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip:sched/urgent] sched/numa: Fix task_numa_free() lockdep splat
2014-04-07 8:55 ` Mike Galbraith
2014-04-13 20:53 ` Govindarajulu Varadarajan
@ 2014-04-14 7:22 ` tip-bot for Mike Galbraith
1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for Mike Galbraith @ 2014-04-14 7:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, sasha.levin, hpa, mingo, peterz, bitbucket, davej, tglx
Commit-ID: 60e69eed85bb7b5198ef70643b5895c26ad76ef7
Gitweb: http://git.kernel.org/tip/60e69eed85bb7b5198ef70643b5895c26ad76ef7
Author: Mike Galbraith <bitbucket@online.de>
AuthorDate: Mon, 7 Apr 2014 10:55:15 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 11 Apr 2014 10:39:15 +0200
sched/numa: Fix task_numa_free() lockdep splat
Sasha reported that lockdep claims that the following commit:
made numa_group.lock interrupt unsafe:
156654f491dd ("sched/numa: Move task_numa_free() to __put_task_struct()")
While I don't see how that could be, given the commit in question moved
task_numa_free() from one irq enabled region to another, the below does
make both gripes and lockups upon gripe with numa=fake=4 go away.
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Fixes: 156654f491dd ("sched/numa: Move task_numa_free() to __put_task_struct()")
Signed-off-by: Mike Galbraith <bitbucket@online.de>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Cc: torvalds@linux-foundation.org
Cc: mgorman@suse.com
Cc: akpm@linux-foundation.org
Cc: Dave Jones <davej@redhat.com>
Link: http://lkml.kernel.org/r/1396860915.5170.5.camel@marge.simpson.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/fair.c | 13 +++++++------
kernel/sched/sched.h | 9 +++++++++
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7e9bd0b..4f14a65 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1497,7 +1497,7 @@ static void task_numa_placement(struct task_struct *p)
/* If the task is part of a group prevent parallel updates to group stats */
if (p->numa_group) {
group_lock = &p->numa_group->lock;
- spin_lock(group_lock);
+ spin_lock_irq(group_lock);
}
/* Find the node with the highest number of faults */
@@ -1572,7 +1572,7 @@ static void task_numa_placement(struct task_struct *p)
}
}
- spin_unlock(group_lock);
+ spin_unlock_irq(group_lock);
}
/* Preferred node as the node with the most faults */
@@ -1677,7 +1677,8 @@ static void task_numa_group(struct task_struct *p, int cpupid, int flags,
if (!join)
return;
- double_lock(&my_grp->lock, &grp->lock);
+ BUG_ON(irqs_disabled());
+ double_lock_irq(&my_grp->lock, &grp->lock);
for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++) {
my_grp->faults[i] -= p->numa_faults_memory[i];
@@ -1691,7 +1692,7 @@ static void task_numa_group(struct task_struct *p, int cpupid, int flags,
grp->nr_tasks++;
spin_unlock(&my_grp->lock);
- spin_unlock(&grp->lock);
+ spin_unlock_irq(&grp->lock);
rcu_assign_pointer(p->numa_group, grp);
@@ -1710,14 +1711,14 @@ void task_numa_free(struct task_struct *p)
void *numa_faults = p->numa_faults_memory;
if (grp) {
- spin_lock(&grp->lock);
+ spin_lock_irq(&grp->lock);
for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++)
grp->faults[i] -= p->numa_faults_memory[i];
grp->total_faults -= p->total_numa_faults;
list_del(&p->numa_entry);
grp->nr_tasks--;
- spin_unlock(&grp->lock);
+ spin_unlock_irq(&grp->lock);
rcu_assign_pointer(p->numa_group, NULL);
put_numa_group(grp);
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index c9007f2..456e492 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1385,6 +1385,15 @@ static inline void double_lock(spinlock_t *l1, spinlock_t *l2)
spin_lock_nested(l2, SINGLE_DEPTH_NESTING);
}
+static inline void double_lock_irq(spinlock_t *l1, spinlock_t *l2)
+{
+ if (l1 > l2)
+ swap(l1, l2);
+
+ spin_lock_irq(l1);
+ spin_lock_nested(l2, SINGLE_DEPTH_NESTING);
+}
+
static inline void double_raw_lock(raw_spinlock_t *l1, raw_spinlock_t *l2)
{
if (l1 > l2)
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-04-14 7:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-28 6:23 [patch] rt,sched,numa: Move task_numa_free() to __put_task_struct(), which -rt offloads Mike Galbraith
2014-02-28 9:00 ` Pavel Vasilyev
2014-02-28 11:32 ` Peter Zijlstra
2014-03-11 12:40 ` [tip:sched/core] sched/numa: Move task_numa_free() to __put_task_struct() tip-bot for Mike Galbraith
2014-04-06 19:17 ` Sasha Levin
2014-04-07 5:29 ` Mike Galbraith
2014-04-07 7:30 ` Mike Galbraith
2014-04-07 8:16 ` Peter Zijlstra
2014-04-07 8:40 ` Mike Galbraith
2014-04-07 8:55 ` Mike Galbraith
2014-04-13 20:53 ` Govindarajulu Varadarajan
2014-04-14 7:22 ` [tip:sched/urgent] sched/numa: Fix task_numa_free() lockdep splat tip-bot for Mike Galbraith
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.