From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754582Ab3INJGD (ORCPT ); Sat, 14 Sep 2013 05:06:03 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:19242 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752923Ab3INJF7 (ORCPT ); Sat, 14 Sep 2013 05:05:59 -0400 Message-ID: <52342687.2060901@huawei.com> Date: Sat, 14 Sep 2013 17:04:07 +0800 From: Li Zefan User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 MIME-Version: 1.0 To: John Stultz CC: lkml , Mathieu Desnoyers , Steven Rostedt , Peter Zijlstra , Ingo Molnar , Thomas Gleixner , Mel Gorman Subject: Re: [PATCH 2/2] [RFC] cpuset: Fix potential deadlock w/ set_mems_allowed References: <1379117948-7385-1-git-send-email-john.stultz@linaro.org> <1379117948-7385-2-git-send-email-john.stultz@linaro.org> In-Reply-To: <1379117948-7385-2-git-send-email-john.stultz@linaro.org> Content-Type: text/plain; charset="GB2312" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.135.68.215] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Cc Mel, who added seqcount to cpuset. On 2013/9/14 8:19, John Stultz wrote: > After adding lockdep support to seqlock/seqcount structures, > I started seeing the following warning: > > [ 1.070907] ====================================================== > [ 1.072015] [ INFO: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected ] > [ 1.073181] 3.11.0+ #67 Not tainted > [ 1.073801] ------------------------------------------------------ > [ 1.074882] kworker/u4:2/708 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire: > [ 1.076088] (&p->mems_allowed_seq){+.+...}, at: [] new_slab+0x5f/0x280 > [ 1.077572] > [ 1.077572] and this task is already holding: > [ 1.078593] (&(&q->__queue_lock)->rlock){..-...}, at: [] blk_execute_rq_nowait+0x53/0xf0 > [ 1.080042] which would create a new lock dependency: > [ 1.080042] (&(&q->__queue_lock)->rlock){..-...} -> (&p->mems_allowed_seq){+.+...} > [ 1.080042] > [ 1.080042] but this new dependency connects a SOFTIRQ-irq-safe lock: > [ 1.080042] (&(&q->__queue_lock)->rlock){..-...} > [ 1.080042] ... which became SOFTIRQ-irq-safe at: > [ 1.080042] [] __lock_acquire+0x5b9/0x1db0 > [ 1.080042] [] lock_acquire+0x95/0x130 > [ 1.080042] [] _raw_spin_lock+0x41/0x80 > [ 1.080042] [] scsi_device_unbusy+0x7e/0xd0 > [ 1.080042] [] scsi_finish_command+0x32/0xf0 > [ 1.080042] [] scsi_softirq_done+0xa1/0x130 > [ 1.080042] [] blk_done_softirq+0x73/0x90 > [ 1.080042] [] __do_softirq+0x110/0x2f0 > [ 1.080042] [] run_ksoftirqd+0x2d/0x60 > [ 1.080042] [] smpboot_thread_fn+0x156/0x1e0 > [ 1.080042] [] kthread+0xd6/0xe0 > [ 1.080042] [] ret_from_fork+0x7c/0xb0 > [ 1.080042] > [ 1.080042] to a SOFTIRQ-irq-unsafe lock: > [ 1.080042] (&p->mems_allowed_seq){+.+...} > [ 1.080042] ... which became SOFTIRQ-irq-unsafe at: > [ 1.080042] ... [] __lock_acquire+0x613/0x1db0 > [ 1.080042] [] lock_acquire+0x95/0x130 > [ 1.080042] [] kthreadd+0x82/0x180 > [ 1.080042] [] ret_from_fork+0x7c/0xb0 > [ 1.080042] > [ 1.080042] other info that might help us debug this: > [ 1.080042] > [ 1.080042] Possible interrupt unsafe locking scenario: > [ 1.080042] > [ 1.080042] CPU0 CPU1 > [ 1.080042] ---- ---- > [ 1.080042] lock(&p->mems_allowed_seq); > [ 1.080042] local_irq_disable(); > [ 1.080042] lock(&(&q->__queue_lock)->rlock); > [ 1.080042] lock(&p->mems_allowed_seq); > [ 1.080042] > [ 1.080042] lock(&(&q->__queue_lock)->rlock); > [ 1.080042] > [ 1.080042] *** DEADLOCK *** > > The issue stems from the kthreadd() function calling set_mems_allowed > with irqs enabled. While its possibly unlikely for the actual deadlock > to trigger, a fix is fairly simple: disable irqs before taking the > mems_allowed_seq lock. > Now I get it. I'm fine with this change. Acked-by: Li Zefan > Let me know if you have any other suggestions or alternative fixes you'd > prefer. > > Cc: Li Zefan > Cc: Mathieu Desnoyers > Cc: Steven Rostedt > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: Thomas Gleixner > Signed-off-by: John Stultz > --- > include/linux/cpuset.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h > index cc1b01c..3fe661f 100644 > --- a/include/linux/cpuset.h > +++ b/include/linux/cpuset.h > @@ -110,10 +110,14 @@ static inline bool put_mems_allowed(unsigned int seq) > > static inline void set_mems_allowed(nodemask_t nodemask) > { > + unsigned long flags; > + > task_lock(current); > + local_irq_save(flags); > write_seqcount_begin(¤t->mems_allowed_seq); > current->mems_allowed = nodemask; > write_seqcount_end(¤t->mems_allowed_seq); > + local_irq_restore(flags); > task_unlock(current); > } > >