From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Snitzer Subject: Re: 3.15-rc4: circular locking dependency triggered by dm-multipath Date: Wed, 7 May 2014 16:53:33 -0400 Message-ID: <20140507205333.GA29666@redhat.com> References: <5368ABE8.9050909@acm.org> Reply-To: device-mapper development Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <5368ABE8.9050909@acm.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Bart Van Assche Cc: device-mapper development List-Id: dm-devel.ids Hi Bart, On Tue, May 06 2014 at 5:31am -0400, Bart Van Assche wrote: > Hello, > > Has anyone else perhaps already run into this ? > > Thanks, > > Bart. > > ====================================================== > [ INFO: possible circular locking dependency detected ] > 3.15.0-rc4-debug+ #1 Not tainted > ------------------------------------------------------- > multipathd/10364 is trying to acquire lock: > (&(&q->__queue_lock)->rlock){-.-...}, at: [] dm_table_run_md_queue_async+0x33/0x60 [dm_mod] > > but task is already holding lock: > (&(&m->lock)->rlock){..-...}, at: [] queue_if_no_path+0x27/0xc0 [dm_multipath] > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&(&m->lock)->rlock){..-...}: > [] lock_acquire+0x93/0x1c0 > [] _raw_spin_lock+0x3b/0x50 > [] dm_blk_open+0x19/0x80 [dm_mod] > [] __blkdev_get+0xd1/0x4c0 > [] blkdev_get+0x1e5/0x380 > [] blkdev_open+0x5b/0x80 > [] do_dentry_open.isra.15+0x1de/0x2a0 > [] finish_open+0x30/0x40 > [] do_last.isra.61+0xa5d/0x1200 > [] path_openat+0xb7/0x620 > [] do_filp_open+0x3a/0x90 > [] do_sys_open+0x12e/0x210 > [] SyS_open+0x1e/0x20 > [] tracesys+0xd0/0xd5 I'm not sure what lockdep is trying to communicate for #1 here. Not seeing where m->lock is taken in the above stack. Nor does it appear to take q->queue_lock. > -> #0 (&(&q->__queue_lock)->rlock){-.-...}: > [] __lock_acquire+0x1716/0x1a00 > [] lock_acquire+0x93/0x1c0 > [] _raw_spin_lock_irqsave+0x46/0x60 > [] dm_table_run_md_queue_async+0x33/0x60 [dm_mod] > [] queue_if_no_path+0x72/0xc0 [dm_multipath] > [] multipath_presuspend+0x19/0x20 [dm_multipath] > [] dm_table_presuspend_targets+0x4a/0x60 [dm_mod] > [] dm_suspend+0x6d/0x1f0 [dm_mod] > [] dev_suspend+0x1c3/0x220 [dm_mod] > [] ctl_ioctl+0x269/0x500 [dm_mod] > [] dm_ctl_ioctl+0x13/0x20 [dm_mod] > [] do_vfs_ioctl+0x300/0x520 > [] SyS_ioctl+0x41/0x80 > [] tracesys+0xd0/0xd5 > > other info that might help us debug this: > > Possible unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(&(&m->lock)->rlock); > lock(&(&q->__queue_lock)->rlock); > lock(&(&m->lock)->rlock); > lock(&(&q->__queue_lock)->rlock); > > *** DEADLOCK *** I'm not seeing where q->queue_lock is held before m->lock is taken. I did find that multipath_ioctl() does _not_ hold m->lock when calling dm_table_run_md_queue_async() -- the other callers hold m->lock. Any chance you can see if this patch silences lockdep? -- drivers/md/dm-mpath.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index aa009e8..fa0f6cb 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -1566,8 +1566,8 @@ static int multipath_ioctl(struct dm_target *ti, unsigned int cmd, } if (m->pg_init_required) __pg_init_all_paths(m); - spin_unlock_irqrestore(&m->lock, flags); dm_table_run_md_queue_async(m->ti->table); + spin_unlock_irqrestore(&m->lock, flags); } return r ? : __blkdev_driver_ioctl(bdev, mode, cmd, arg);