Re: [PATCH] cpuset: Fix memory allocator deadlock

2013-11-27 Thread Tejun Heo
On Tue, Nov 26, 2013 at 03:03:41PM +0100, Peter Zijlstra wrote:
> Juri hit the below lockdep report:
> 
> [4.303391] ==
> [4.303392] [ INFO: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected ]
> [4.303394] 3.12.0-dl-peterz+ #144 Not tainted
> [4.303395] --
> [4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> [4.303399]  (>mems_allowed_seq){+.+...}, at: [] 
> new_slab+0x6c/0x290
> [4.303417]
> [4.303417] and this task is already holding:
> [4.303418]  (&(>__queue_lock)->rlock){..-...}, at: 
> [] blk_execute_rq_nowait+0x5b/0x100
> [4.303431] which would create a new lock dependency:
> [4.303432]  (&(>__queue_lock)->rlock){..-...} -> 
> (>mems_allowed_seq){+.+...}
> [4.303436]
> 
> [4.303898] the dependencies between the lock to be acquired and 
> SOFTIRQ-irq-unsafe lock:
> [4.303918] -> (>mems_allowed_seq){+.+...} ops: 2762 {
> [4.303922]HARDIRQ-ON-W at:
> [4.303923] [] 
> __lock_acquire+0x65a/0x1ff0
> [4.303926] [] 
> lock_acquire+0x93/0x140
> [4.303929] [] kthreadd+0x86/0x180
> [4.303931] [] 
> ret_from_fork+0x7c/0xb0
> [4.303933]SOFTIRQ-ON-W at:
> [4.303933] [] 
> __lock_acquire+0x68c/0x1ff0
> [4.303935] [] 
> lock_acquire+0x93/0x140
> [4.303940] [] kthreadd+0x86/0x180
> [4.303955] [] 
> ret_from_fork+0x7c/0xb0
> [4.303959]INITIAL USE at:
> [4.303960][] 
> __lock_acquire+0x344/0x1ff0
> [4.303963][] lock_acquire+0x93/0x140
> [4.303966][] kthreadd+0x86/0x180
> [4.303969][] ret_from_fork+0x7c/0xb0
> [4.303972]  }
> 
> Which reports that we take mems_allowed_seq with interrupts enabled. A
> little digging found that this can only be from
> cpuset_change_task_nodemask().
> 
> This is an actual deadlock because an interrupt doing an allocation will
> hit get_mems_allowed()->...->__read_seqcount_begin(), which will spin
> forever waiting for the write side to complete.
> 
> Cc: John Stultz 
> Cc: Mel Gorman 
> Reported-by: Juri Lelli 
> Signed-off-by: Peter Zijlstra 

Applied to cgroup/for-3.13-fixes w/ stable cc'd.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuset: Fix memory allocator deadlock

2013-11-27 Thread Mel Gorman
On Tue, Nov 26, 2013 at 03:03:41PM +0100, Peter Zijlstra wrote:
> Juri hit the below lockdep report:
> 
> [4.303391] ==
> [4.303392] [ INFO: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected ]
> [4.303394] 3.12.0-dl-peterz+ #144 Not tainted
> [4.303395] --
> [4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> [4.303399]  (>mems_allowed_seq){+.+...}, at: [] 
> new_slab+0x6c/0x290
> [4.303417]
> [4.303417] and this task is already holding:
> [4.303418]  (&(>__queue_lock)->rlock){..-...}, at: 
> [] blk_execute_rq_nowait+0x5b/0x100
> [4.303431] which would create a new lock dependency:
> [4.303432]  (&(>__queue_lock)->rlock){..-...} -> 
> (>mems_allowed_seq){+.+...}
> [4.303436]
> 
> [4.303898] the dependencies between the lock to be acquired and 
> SOFTIRQ-irq-unsafe lock:
> [4.303918] -> (>mems_allowed_seq){+.+...} ops: 2762 {
> [4.303922]HARDIRQ-ON-W at:
> [4.303923] [] 
> __lock_acquire+0x65a/0x1ff0
> [4.303926] [] 
> lock_acquire+0x93/0x140
> [4.303929] [] kthreadd+0x86/0x180
> [4.303931] [] 
> ret_from_fork+0x7c/0xb0
> [4.303933]SOFTIRQ-ON-W at:
> [4.303933] [] 
> __lock_acquire+0x68c/0x1ff0
> [4.303935] [] 
> lock_acquire+0x93/0x140
> [4.303940] [] kthreadd+0x86/0x180
> [4.303955] [] 
> ret_from_fork+0x7c/0xb0
> [4.303959]INITIAL USE at:
> [4.303960][] 
> __lock_acquire+0x344/0x1ff0
> [4.303963][] lock_acquire+0x93/0x140
> [4.303966][] kthreadd+0x86/0x180
> [4.303969][] ret_from_fork+0x7c/0xb0
> [4.303972]  }
> 
> Which reports that we take mems_allowed_seq with interrupts enabled. A
> little digging found that this can only be from
> cpuset_change_task_nodemask().
> 
> This is an actual deadlock because an interrupt doing an allocation will
> hit get_mems_allowed()->...->__read_seqcount_begin(), which will spin
> forever waiting for the write side to complete.
> 
> Cc: John Stultz 
> Cc: Mel Gorman 
> Reported-by: Juri Lelli 
> Signed-off-by: Peter Zijlstra 

Acked-by: Mel Gorman 

-- 
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuset: Fix memory allocator deadlock

2013-11-27 Thread Mel Gorman
On Tue, Nov 26, 2013 at 03:03:41PM +0100, Peter Zijlstra wrote:
 Juri hit the below lockdep report:
 
 [4.303391] ==
 [4.303392] [ INFO: SOFTIRQ-safe - SOFTIRQ-unsafe lock order detected ]
 [4.303394] 3.12.0-dl-peterz+ #144 Not tainted
 [4.303395] --
 [4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
 [4.303399]  (p-mems_allowed_seq){+.+...}, at: [8114e63c] 
 new_slab+0x6c/0x290
 [4.303417]
 [4.303417] and this task is already holding:
 [4.303418]  ((q-__queue_lock)-rlock){..-...}, at: 
 [812d2dfb] blk_execute_rq_nowait+0x5b/0x100
 [4.303431] which would create a new lock dependency:
 [4.303432]  ((q-__queue_lock)-rlock){..-...} - 
 (p-mems_allowed_seq){+.+...}
 [4.303436]
 
 [4.303898] the dependencies between the lock to be acquired and 
 SOFTIRQ-irq-unsafe lock:
 [4.303918] - (p-mems_allowed_seq){+.+...} ops: 2762 {
 [4.303922]HARDIRQ-ON-W at:
 [4.303923] [8108ab9a] 
 __lock_acquire+0x65a/0x1ff0
 [4.303926] [8108cbe3] 
 lock_acquire+0x93/0x140
 [4.303929] [81063dd6] kthreadd+0x86/0x180
 [4.303931] [816ded6c] 
 ret_from_fork+0x7c/0xb0
 [4.303933]SOFTIRQ-ON-W at:
 [4.303933] [8108abcc] 
 __lock_acquire+0x68c/0x1ff0
 [4.303935] [8108cbe3] 
 lock_acquire+0x93/0x140
 [4.303940] [81063dd6] kthreadd+0x86/0x180
 [4.303955] [816ded6c] 
 ret_from_fork+0x7c/0xb0
 [4.303959]INITIAL USE at:
 [4.303960][8108a884] 
 __lock_acquire+0x344/0x1ff0
 [4.303963][8108cbe3] lock_acquire+0x93/0x140
 [4.303966][81063dd6] kthreadd+0x86/0x180
 [4.303969][816ded6c] ret_from_fork+0x7c/0xb0
 [4.303972]  }
 
 Which reports that we take mems_allowed_seq with interrupts enabled. A
 little digging found that this can only be from
 cpuset_change_task_nodemask().
 
 This is an actual deadlock because an interrupt doing an allocation will
 hit get_mems_allowed()-...-__read_seqcount_begin(), which will spin
 forever waiting for the write side to complete.
 
 Cc: John Stultz john.stu...@linaro.org
 Cc: Mel Gorman mgor...@suse.de
 Reported-by: Juri Lelli juri.le...@gmail.com
 Signed-off-by: Peter Zijlstra pet...@infradead.org

Acked-by: Mel Gorman mgor...@suse.de

-- 
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuset: Fix memory allocator deadlock

2013-11-27 Thread Tejun Heo
On Tue, Nov 26, 2013 at 03:03:41PM +0100, Peter Zijlstra wrote:
 Juri hit the below lockdep report:
 
 [4.303391] ==
 [4.303392] [ INFO: SOFTIRQ-safe - SOFTIRQ-unsafe lock order detected ]
 [4.303394] 3.12.0-dl-peterz+ #144 Not tainted
 [4.303395] --
 [4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
 [4.303399]  (p-mems_allowed_seq){+.+...}, at: [8114e63c] 
 new_slab+0x6c/0x290
 [4.303417]
 [4.303417] and this task is already holding:
 [4.303418]  ((q-__queue_lock)-rlock){..-...}, at: 
 [812d2dfb] blk_execute_rq_nowait+0x5b/0x100
 [4.303431] which would create a new lock dependency:
 [4.303432]  ((q-__queue_lock)-rlock){..-...} - 
 (p-mems_allowed_seq){+.+...}
 [4.303436]
 
 [4.303898] the dependencies between the lock to be acquired and 
 SOFTIRQ-irq-unsafe lock:
 [4.303918] - (p-mems_allowed_seq){+.+...} ops: 2762 {
 [4.303922]HARDIRQ-ON-W at:
 [4.303923] [8108ab9a] 
 __lock_acquire+0x65a/0x1ff0
 [4.303926] [8108cbe3] 
 lock_acquire+0x93/0x140
 [4.303929] [81063dd6] kthreadd+0x86/0x180
 [4.303931] [816ded6c] 
 ret_from_fork+0x7c/0xb0
 [4.303933]SOFTIRQ-ON-W at:
 [4.303933] [8108abcc] 
 __lock_acquire+0x68c/0x1ff0
 [4.303935] [8108cbe3] 
 lock_acquire+0x93/0x140
 [4.303940] [81063dd6] kthreadd+0x86/0x180
 [4.303955] [816ded6c] 
 ret_from_fork+0x7c/0xb0
 [4.303959]INITIAL USE at:
 [4.303960][8108a884] 
 __lock_acquire+0x344/0x1ff0
 [4.303963][8108cbe3] lock_acquire+0x93/0x140
 [4.303966][81063dd6] kthreadd+0x86/0x180
 [4.303969][816ded6c] ret_from_fork+0x7c/0xb0
 [4.303972]  }
 
 Which reports that we take mems_allowed_seq with interrupts enabled. A
 little digging found that this can only be from
 cpuset_change_task_nodemask().
 
 This is an actual deadlock because an interrupt doing an allocation will
 hit get_mems_allowed()-...-__read_seqcount_begin(), which will spin
 forever waiting for the write side to complete.
 
 Cc: John Stultz john.stu...@linaro.org
 Cc: Mel Gorman mgor...@suse.de
 Reported-by: Juri Lelli juri.le...@gmail.com
 Signed-off-by: Peter Zijlstra pet...@infradead.org

Applied to cgroup/for-3.13-fixes w/ stable cc'd.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuset: Fix memory allocator deadlock

2013-11-26 Thread Li Zefan
On 2013/11/26 22:03, Peter Zijlstra wrote:
> Juri hit the below lockdep report:
> 
> [4.303391] ==
> [4.303392] [ INFO: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected ]
> [4.303394] 3.12.0-dl-peterz+ #144 Not tainted
> [4.303395] --
> [4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> [4.303399]  (>mems_allowed_seq){+.+...}, at: [] 
> new_slab+0x6c/0x290
> [4.303417]
> [4.303417] and this task is already holding:
> [4.303418]  (&(>__queue_lock)->rlock){..-...}, at: 
> [] blk_execute_rq_nowait+0x5b/0x100
> [4.303431] which would create a new lock dependency:
> [4.303432]  (&(>__queue_lock)->rlock){..-...} -> 
> (>mems_allowed_seq){+.+...}
> [4.303436]
> 
> [4.303898] the dependencies between the lock to be acquired and 
> SOFTIRQ-irq-unsafe lock:
> [4.303918] -> (>mems_allowed_seq){+.+...} ops: 2762 {
> [4.303922]HARDIRQ-ON-W at:
> [4.303923] [] 
> __lock_acquire+0x65a/0x1ff0
> [4.303926] [] 
> lock_acquire+0x93/0x140
> [4.303929] [] kthreadd+0x86/0x180
> [4.303931] [] 
> ret_from_fork+0x7c/0xb0
> [4.303933]SOFTIRQ-ON-W at:
> [4.303933] [] 
> __lock_acquire+0x68c/0x1ff0
> [4.303935] [] 
> lock_acquire+0x93/0x140
> [4.303940] [] kthreadd+0x86/0x180
> [4.303955] [] 
> ret_from_fork+0x7c/0xb0
> [4.303959]INITIAL USE at:
> [4.303960][] 
> __lock_acquire+0x344/0x1ff0
> [4.303963][] lock_acquire+0x93/0x140
> [4.303966][] kthreadd+0x86/0x180
> [4.303969][] ret_from_fork+0x7c/0xb0
> [4.303972]  }
> 
> Which reports that we take mems_allowed_seq with interrupts enabled. A
> little digging found that this can only be from
> cpuset_change_task_nodemask().
> 

Yeah, the other one in set_mems_allowed() was fixed by John.

> This is an actual deadlock because an interrupt doing an allocation will
> hit get_mems_allowed()->...->__read_seqcount_begin(), which will spin
> forever waiting for the write side to complete.
> 
> Cc: John Stultz 
> Cc: Mel Gorman 
> Reported-by: Juri Lelli 
> Signed-off-by: Peter Zijlstra 

Acked-by: Li Zefan 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuset: Fix memory allocator deadlock

2013-11-26 Thread Juri Lelli
On 11/26/2013 03:03 PM, Peter Zijlstra wrote:
> Juri hit the below lockdep report:
> 
> [4.303391] ==
> [4.303392] [ INFO: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected ]
> [4.303394] 3.12.0-dl-peterz+ #144 Not tainted
> [4.303395] --
> [4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> [4.303399]  (>mems_allowed_seq){+.+...}, at: [] 
> new_slab+0x6c/0x290
> [4.303417]
> [4.303417] and this task is already holding:
> [4.303418]  (&(>__queue_lock)->rlock){..-...}, at: 
> [] blk_execute_rq_nowait+0x5b/0x100
> [4.303431] which would create a new lock dependency:
> [4.303432]  (&(>__queue_lock)->rlock){..-...} -> 
> (>mems_allowed_seq){+.+...}
> [4.303436]
> 
> [4.303898] the dependencies between the lock to be acquired and 
> SOFTIRQ-irq-unsafe lock:
> [4.303918] -> (>mems_allowed_seq){+.+...} ops: 2762 {
> [4.303922]HARDIRQ-ON-W at:
> [4.303923] [] 
> __lock_acquire+0x65a/0x1ff0
> [4.303926] [] 
> lock_acquire+0x93/0x140
> [4.303929] [] kthreadd+0x86/0x180
> [4.303931] [] 
> ret_from_fork+0x7c/0xb0
> [4.303933]SOFTIRQ-ON-W at:
> [4.303933] [] 
> __lock_acquire+0x68c/0x1ff0
> [4.303935] [] 
> lock_acquire+0x93/0x140
> [4.303940] [] kthreadd+0x86/0x180
> [4.303955] [] 
> ret_from_fork+0x7c/0xb0
> [4.303959]INITIAL USE at:
> [4.303960][] 
> __lock_acquire+0x344/0x1ff0
> [4.303963][] lock_acquire+0x93/0x140
> [4.303966][] kthreadd+0x86/0x180
> [4.303969][] ret_from_fork+0x7c/0xb0
> [4.303972]  }
> 
> Which reports that we take mems_allowed_seq with interrupts enabled. A
> little digging found that this can only be from
> cpuset_change_task_nodemask().
> 
> This is an actual deadlock because an interrupt doing an allocation will
> hit get_mems_allowed()->...->__read_seqcount_begin(), which will spin
> forever waiting for the write side to complete.
> 

And this patch fixes it, thanks!

> Cc: John Stultz 
> Cc: Mel Gorman 
> Reported-by: Juri Lelli 
> Signed-off-by: Peter Zijlstra 

Tested-by: Juri Lelli 

Best,

- Juri

> ---
>  kernel/cpuset.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index 6bf981e13c43..4772034b4b17 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -1033,8 +1033,10 @@ static void cpuset_change_task_nodemask(struct 
> task_struct *tsk,
>   need_loop = task_has_mempolicy(tsk) ||
>   !nodes_intersects(*newmems, tsk->mems_allowed);
>  
> - if (need_loop)
> + if (need_loop) {
> + local_irq_disable();
>   write_seqcount_begin(>mems_allowed_seq);
> + }
>  
>   nodes_or(tsk->mems_allowed, tsk->mems_allowed, *newmems);
>   mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP1);
> @@ -1042,8 +1044,10 @@ static void cpuset_change_task_nodemask(struct 
> task_struct *tsk,
>   mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP2);
>   tsk->mems_allowed = *newmems;
>  
> - if (need_loop)
> + if (need_loop) {
>   write_seqcount_end(>mems_allowed_seq);
> + local_irq_enable();
> + }
>  
>   task_unlock(tsk);
>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] cpuset: Fix memory allocator deadlock

2013-11-26 Thread Peter Zijlstra
Juri hit the below lockdep report:

[4.303391] ==
[4.303392] [ INFO: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected ]
[4.303394] 3.12.0-dl-peterz+ #144 Not tainted
[4.303395] --
[4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[4.303399]  (>mems_allowed_seq){+.+...}, at: [] 
new_slab+0x6c/0x290
[4.303417]
[4.303417] and this task is already holding:
[4.303418]  (&(>__queue_lock)->rlock){..-...}, at: [] 
blk_execute_rq_nowait+0x5b/0x100
[4.303431] which would create a new lock dependency:
[4.303432]  (&(>__queue_lock)->rlock){..-...} -> 
(>mems_allowed_seq){+.+...}
[4.303436]

[4.303898] the dependencies between the lock to be acquired and 
SOFTIRQ-irq-unsafe lock:
[4.303918] -> (>mems_allowed_seq){+.+...} ops: 2762 {
[4.303922]HARDIRQ-ON-W at:
[4.303923] [] 
__lock_acquire+0x65a/0x1ff0
[4.303926] [] lock_acquire+0x93/0x140
[4.303929] [] kthreadd+0x86/0x180
[4.303931] [] ret_from_fork+0x7c/0xb0
[4.303933]SOFTIRQ-ON-W at:
[4.303933] [] 
__lock_acquire+0x68c/0x1ff0
[4.303935] [] lock_acquire+0x93/0x140
[4.303940] [] kthreadd+0x86/0x180
[4.303955] [] ret_from_fork+0x7c/0xb0
[4.303959]INITIAL USE at:
[4.303960][] 
__lock_acquire+0x344/0x1ff0
[4.303963][] lock_acquire+0x93/0x140
[4.303966][] kthreadd+0x86/0x180
[4.303969][] ret_from_fork+0x7c/0xb0
[4.303972]  }

Which reports that we take mems_allowed_seq with interrupts enabled. A
little digging found that this can only be from
cpuset_change_task_nodemask().

This is an actual deadlock because an interrupt doing an allocation will
hit get_mems_allowed()->...->__read_seqcount_begin(), which will spin
forever waiting for the write side to complete.

Cc: John Stultz 
Cc: Mel Gorman 
Reported-by: Juri Lelli 
Signed-off-by: Peter Zijlstra 
---
 kernel/cpuset.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 6bf981e13c43..4772034b4b17 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1033,8 +1033,10 @@ static void cpuset_change_task_nodemask(struct 
task_struct *tsk,
need_loop = task_has_mempolicy(tsk) ||
!nodes_intersects(*newmems, tsk->mems_allowed);
 
-   if (need_loop)
+   if (need_loop) {
+   local_irq_disable();
write_seqcount_begin(>mems_allowed_seq);
+   }
 
nodes_or(tsk->mems_allowed, tsk->mems_allowed, *newmems);
mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP1);
@@ -1042,8 +1044,10 @@ static void cpuset_change_task_nodemask(struct 
task_struct *tsk,
mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP2);
tsk->mems_allowed = *newmems;
 
-   if (need_loop)
+   if (need_loop) {
write_seqcount_end(>mems_allowed_seq);
+   local_irq_enable();
+   }
 
task_unlock(tsk);
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] cpuset: Fix memory allocator deadlock

2013-11-26 Thread Peter Zijlstra
Juri hit the below lockdep report:

[4.303391] ==
[4.303392] [ INFO: SOFTIRQ-safe - SOFTIRQ-unsafe lock order detected ]
[4.303394] 3.12.0-dl-peterz+ #144 Not tainted
[4.303395] --
[4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[4.303399]  (p-mems_allowed_seq){+.+...}, at: [8114e63c] 
new_slab+0x6c/0x290
[4.303417]
[4.303417] and this task is already holding:
[4.303418]  ((q-__queue_lock)-rlock){..-...}, at: [812d2dfb] 
blk_execute_rq_nowait+0x5b/0x100
[4.303431] which would create a new lock dependency:
[4.303432]  ((q-__queue_lock)-rlock){..-...} - 
(p-mems_allowed_seq){+.+...}
[4.303436]

[4.303898] the dependencies between the lock to be acquired and 
SOFTIRQ-irq-unsafe lock:
[4.303918] - (p-mems_allowed_seq){+.+...} ops: 2762 {
[4.303922]HARDIRQ-ON-W at:
[4.303923] [8108ab9a] 
__lock_acquire+0x65a/0x1ff0
[4.303926] [8108cbe3] lock_acquire+0x93/0x140
[4.303929] [81063dd6] kthreadd+0x86/0x180
[4.303931] [816ded6c] ret_from_fork+0x7c/0xb0
[4.303933]SOFTIRQ-ON-W at:
[4.303933] [8108abcc] 
__lock_acquire+0x68c/0x1ff0
[4.303935] [8108cbe3] lock_acquire+0x93/0x140
[4.303940] [81063dd6] kthreadd+0x86/0x180
[4.303955] [816ded6c] ret_from_fork+0x7c/0xb0
[4.303959]INITIAL USE at:
[4.303960][8108a884] 
__lock_acquire+0x344/0x1ff0
[4.303963][8108cbe3] lock_acquire+0x93/0x140
[4.303966][81063dd6] kthreadd+0x86/0x180
[4.303969][816ded6c] ret_from_fork+0x7c/0xb0
[4.303972]  }

Which reports that we take mems_allowed_seq with interrupts enabled. A
little digging found that this can only be from
cpuset_change_task_nodemask().

This is an actual deadlock because an interrupt doing an allocation will
hit get_mems_allowed()-...-__read_seqcount_begin(), which will spin
forever waiting for the write side to complete.

Cc: John Stultz john.stu...@linaro.org
Cc: Mel Gorman mgor...@suse.de
Reported-by: Juri Lelli juri.le...@gmail.com
Signed-off-by: Peter Zijlstra pet...@infradead.org
---
 kernel/cpuset.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 6bf981e13c43..4772034b4b17 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1033,8 +1033,10 @@ static void cpuset_change_task_nodemask(struct 
task_struct *tsk,
need_loop = task_has_mempolicy(tsk) ||
!nodes_intersects(*newmems, tsk-mems_allowed);
 
-   if (need_loop)
+   if (need_loop) {
+   local_irq_disable();
write_seqcount_begin(tsk-mems_allowed_seq);
+   }
 
nodes_or(tsk-mems_allowed, tsk-mems_allowed, *newmems);
mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP1);
@@ -1042,8 +1044,10 @@ static void cpuset_change_task_nodemask(struct 
task_struct *tsk,
mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP2);
tsk-mems_allowed = *newmems;
 
-   if (need_loop)
+   if (need_loop) {
write_seqcount_end(tsk-mems_allowed_seq);
+   local_irq_enable();
+   }
 
task_unlock(tsk);
 }
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuset: Fix memory allocator deadlock

2013-11-26 Thread Juri Lelli
On 11/26/2013 03:03 PM, Peter Zijlstra wrote:
 Juri hit the below lockdep report:
 
 [4.303391] ==
 [4.303392] [ INFO: SOFTIRQ-safe - SOFTIRQ-unsafe lock order detected ]
 [4.303394] 3.12.0-dl-peterz+ #144 Not tainted
 [4.303395] --
 [4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
 [4.303399]  (p-mems_allowed_seq){+.+...}, at: [8114e63c] 
 new_slab+0x6c/0x290
 [4.303417]
 [4.303417] and this task is already holding:
 [4.303418]  ((q-__queue_lock)-rlock){..-...}, at: 
 [812d2dfb] blk_execute_rq_nowait+0x5b/0x100
 [4.303431] which would create a new lock dependency:
 [4.303432]  ((q-__queue_lock)-rlock){..-...} - 
 (p-mems_allowed_seq){+.+...}
 [4.303436]
 
 [4.303898] the dependencies between the lock to be acquired and 
 SOFTIRQ-irq-unsafe lock:
 [4.303918] - (p-mems_allowed_seq){+.+...} ops: 2762 {
 [4.303922]HARDIRQ-ON-W at:
 [4.303923] [8108ab9a] 
 __lock_acquire+0x65a/0x1ff0
 [4.303926] [8108cbe3] 
 lock_acquire+0x93/0x140
 [4.303929] [81063dd6] kthreadd+0x86/0x180
 [4.303931] [816ded6c] 
 ret_from_fork+0x7c/0xb0
 [4.303933]SOFTIRQ-ON-W at:
 [4.303933] [8108abcc] 
 __lock_acquire+0x68c/0x1ff0
 [4.303935] [8108cbe3] 
 lock_acquire+0x93/0x140
 [4.303940] [81063dd6] kthreadd+0x86/0x180
 [4.303955] [816ded6c] 
 ret_from_fork+0x7c/0xb0
 [4.303959]INITIAL USE at:
 [4.303960][8108a884] 
 __lock_acquire+0x344/0x1ff0
 [4.303963][8108cbe3] lock_acquire+0x93/0x140
 [4.303966][81063dd6] kthreadd+0x86/0x180
 [4.303969][816ded6c] ret_from_fork+0x7c/0xb0
 [4.303972]  }
 
 Which reports that we take mems_allowed_seq with interrupts enabled. A
 little digging found that this can only be from
 cpuset_change_task_nodemask().
 
 This is an actual deadlock because an interrupt doing an allocation will
 hit get_mems_allowed()-...-__read_seqcount_begin(), which will spin
 forever waiting for the write side to complete.
 

And this patch fixes it, thanks!

 Cc: John Stultz john.stu...@linaro.org
 Cc: Mel Gorman mgor...@suse.de
 Reported-by: Juri Lelli juri.le...@gmail.com
 Signed-off-by: Peter Zijlstra pet...@infradead.org

Tested-by: Juri Lelli juri.le...@gmail.com

Best,

- Juri

 ---
  kernel/cpuset.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/kernel/cpuset.c b/kernel/cpuset.c
 index 6bf981e13c43..4772034b4b17 100644
 --- a/kernel/cpuset.c
 +++ b/kernel/cpuset.c
 @@ -1033,8 +1033,10 @@ static void cpuset_change_task_nodemask(struct 
 task_struct *tsk,
   need_loop = task_has_mempolicy(tsk) ||
   !nodes_intersects(*newmems, tsk-mems_allowed);
  
 - if (need_loop)
 + if (need_loop) {
 + local_irq_disable();
   write_seqcount_begin(tsk-mems_allowed_seq);
 + }
  
   nodes_or(tsk-mems_allowed, tsk-mems_allowed, *newmems);
   mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP1);
 @@ -1042,8 +1044,10 @@ static void cpuset_change_task_nodemask(struct 
 task_struct *tsk,
   mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP2);
   tsk-mems_allowed = *newmems;
  
 - if (need_loop)
 + if (need_loop) {
   write_seqcount_end(tsk-mems_allowed_seq);
 + local_irq_enable();
 + }
  
   task_unlock(tsk);
  }
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuset: Fix memory allocator deadlock

2013-11-26 Thread Li Zefan
On 2013/11/26 22:03, Peter Zijlstra wrote:
 Juri hit the below lockdep report:
 
 [4.303391] ==
 [4.303392] [ INFO: SOFTIRQ-safe - SOFTIRQ-unsafe lock order detected ]
 [4.303394] 3.12.0-dl-peterz+ #144 Not tainted
 [4.303395] --
 [4.303397] kworker/u4:3/689 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
 [4.303399]  (p-mems_allowed_seq){+.+...}, at: [8114e63c] 
 new_slab+0x6c/0x290
 [4.303417]
 [4.303417] and this task is already holding:
 [4.303418]  ((q-__queue_lock)-rlock){..-...}, at: 
 [812d2dfb] blk_execute_rq_nowait+0x5b/0x100
 [4.303431] which would create a new lock dependency:
 [4.303432]  ((q-__queue_lock)-rlock){..-...} - 
 (p-mems_allowed_seq){+.+...}
 [4.303436]
 
 [4.303898] the dependencies between the lock to be acquired and 
 SOFTIRQ-irq-unsafe lock:
 [4.303918] - (p-mems_allowed_seq){+.+...} ops: 2762 {
 [4.303922]HARDIRQ-ON-W at:
 [4.303923] [8108ab9a] 
 __lock_acquire+0x65a/0x1ff0
 [4.303926] [8108cbe3] 
 lock_acquire+0x93/0x140
 [4.303929] [81063dd6] kthreadd+0x86/0x180
 [4.303931] [816ded6c] 
 ret_from_fork+0x7c/0xb0
 [4.303933]SOFTIRQ-ON-W at:
 [4.303933] [8108abcc] 
 __lock_acquire+0x68c/0x1ff0
 [4.303935] [8108cbe3] 
 lock_acquire+0x93/0x140
 [4.303940] [81063dd6] kthreadd+0x86/0x180
 [4.303955] [816ded6c] 
 ret_from_fork+0x7c/0xb0
 [4.303959]INITIAL USE at:
 [4.303960][8108a884] 
 __lock_acquire+0x344/0x1ff0
 [4.303963][8108cbe3] lock_acquire+0x93/0x140
 [4.303966][81063dd6] kthreadd+0x86/0x180
 [4.303969][816ded6c] ret_from_fork+0x7c/0xb0
 [4.303972]  }
 
 Which reports that we take mems_allowed_seq with interrupts enabled. A
 little digging found that this can only be from
 cpuset_change_task_nodemask().
 

Yeah, the other one in set_mems_allowed() was fixed by John.

 This is an actual deadlock because an interrupt doing an allocation will
 hit get_mems_allowed()-...-__read_seqcount_begin(), which will spin
 forever waiting for the write side to complete.
 
 Cc: John Stultz john.stu...@linaro.org
 Cc: Mel Gorman mgor...@suse.de
 Reported-by: Juri Lelli juri.le...@gmail.com
 Signed-off-by: Peter Zijlstra pet...@infradead.org

Acked-by: Li Zefan lize...@huawei.com


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/