Date:	Thu, 14 Nov 2013 20:19:16 -0500
From:	Dave Jones <davej@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	Al Viro <viro@...IV.linux.org.uk>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: sysfs_bin_mmap lockdep trace.

On Thu, Nov 14, 2013 at 02:41:16PM +0900, Tejun Heo wrote:
 > hey,
 > 
 > On Wed, Nov 13, 2013 at 08:10:43PM +0000, Al Viro wrote:
 > > On Wed, Nov 13, 2013 at 01:45:38PM -0500, Dave Jones wrote:
 > > > Al, is this one also known ? Also seen on v3.12-7033-g42a2d923cc34
 > > 
 > > Umm...  I've seen something like that reported after sysfs merge went in
 > > (right after 3.12), but I hadn't looked into details.
 > > 
 > > > -> #3 (&mm->mmap_sem){++++++}:
 > > 
 > > 	[sr_block_ioctl() grabs sr_mutex and does copy_from_user() under it]
 > > 
 > > > -> #2 (sr_mutex){+.+.+.}:
 > > 	[sr_block_open() grabs sr_mutex under ->bd_mutex]
 > > 
 > > > -> #1 (&bdev->bd_mutex){+.+.+.}:
 > > 	[sysfs_blk_trace_attr_show() grabs ->bd_mutex and is called under
 > > sysfs_open_file ->mutex]
 > > 
 > > > -> #0 (&of->mutex){+.+.+.}:
 > > 	[sysfs_open_file ->mutex is grabbed by ->mmap()]
 > > 
 > > Cute...  AFAICS, it came from "sysfs: copy bin mmap support from fs/sysfs/bin.c
 > > to fs/sysfs/file.c".  The first impression is that sysfs_bin_mmap() is
 > > checking for battr->mmap too late, but I'm not sure whether we need of->mutex
 > > to stabilize it...  Tejun, any comments?
 > 
 > Hmmm... so this is a false positive from regular and bin file paths
 > being merged.  There was a sysfs regular file which grabbed sr_mutex
 > while holding sysfs mutex and only bin files supported mmap which of
 > course nest under mmap_sem.  As the two paths were separate and using
 > separate locks, this deadlock scenario didn't trigger.  Now that the
 > two paths are merged, lockdep considers the two paths to be using the
 > same mutex (they're per-file so still actually separate) and generates
 > this warning.  The easiest way out would be giving different lock
 > subclasses to files w/ and w/o mmap method.  I'll think more about it.

Tejun,

Is this another variant of the above, or something different ?

	Dave


[  218.248982] ======================================================
[  218.249006] [ INFO: possible circular locking dependency detected ]
[  218.249031] 3.12.0+ #8 Not tainted
[  218.249607] -------------------------------------------------------
[  218.250194] trinity-child1/2100 is trying to acquire lock:
[  218.250784]  (&bdev->bd_mutex){+.+.+.}, at: [<ffffffff8112f89f>] sysfs_blk_trace_attr_show+0x5f/0x1f0
[  218.251408] 
but task is already holding lock:
[  218.252615]  (&of->mutex){+.+.+.}, at: [<ffffffff8124083f>] sysfs_seq_show+0x7f/0x160
[  218.253249] 
which lock already depends on the new lock.

[  218.255178] 
the existing dependency chain (in reverse order) is:
[  218.256480] 
-> #3 (&of->mutex){+.+.+.}:
[  218.257853]        [<ffffffff810af623>] lock_acquire+0x93/0x1c0
[  218.258557]        [<ffffffff81725f67>] mutex_lock_nested+0x77/0x400
[  218.259263]        [<ffffffff8123ffef>] sysfs_bin_mmap+0x4f/0x120
[  218.259900]        [<ffffffff81182695>] mmap_region+0x3e5/0x5d0
[  218.260526]        [<ffffffff81182bd7>] do_mmap_pgoff+0x357/0x3e0
[  218.261149]        [<ffffffff8116d150>] vm_mmap_pgoff+0x90/0xc0
[  218.261765]        [<ffffffff81181125>] SyS_mmap_pgoff+0x1d5/0x270
[  218.262374]        [<ffffffff81007ec2>] SyS_mmap+0x22/0x30
[  218.262983]        [<ffffffff81732ce4>] tracesys+0xdd/0xe2
[  218.263581] 
-> #2 (&mm->mmap_sem){++++++}:
[  218.264853]        [<ffffffff810af623>] lock_acquire+0x93/0x1c0
[  218.265464]        [<ffffffff8117762c>] might_fault+0x8c/0xb0
[  218.266114]        [<ffffffff81302ed5>] scsi_cmd_ioctl+0x295/0x470
[  218.266760]        [<ffffffff813030f2>] scsi_cmd_blk_ioctl+0x42/0x50
[  218.267405]        [<ffffffff8150b5e1>] cdrom_ioctl+0x41/0x1050
[  218.268056]        [<ffffffff814de69f>] sr_block_ioctl+0x6f/0xd0
[  218.268724]        [<ffffffff812fede4>] blkdev_ioctl+0x234/0x840
[  218.269373]        [<ffffffff811faa27>] block_ioctl+0x47/0x50
[  218.270011]        [<ffffffff811ce4f0>] do_vfs_ioctl+0x300/0x520
[  218.270651]        [<ffffffff811ce791>] SyS_ioctl+0x81/0xa0
[  218.271282]        [<ffffffff81732ce4>] tracesys+0xdd/0xe2
[  218.271913] 
-> #1 (sr_mutex){+.+.+.}:
[  218.273153]        [<ffffffff810af623>] lock_acquire+0x93/0x1c0
[  218.273795]        [<ffffffff81725f67>] mutex_lock_nested+0x77/0x400
[  218.274443]        [<ffffffff814ded34>] sr_block_open+0x24/0x130
[  218.275084]        [<ffffffff811fb7f1>] __blkdev_get+0xd1/0x4c0
[  218.275707]        [<ffffffff811fbdc5>] blkdev_get+0x1e5/0x380
[  218.276314]        [<ffffffff811fc01a>] blkdev_open+0x6a/0x90
[  218.276904]        [<ffffffff811b6f97>] do_dentry_open+0x1e7/0x340
[  218.277490]        [<ffffffff811b7200>] finish_open+0x40/0x50
[  218.278070]        [<ffffffff811ca207>] do_last+0xbc7/0x1370
[  218.278637]        [<ffffffff811caa6e>] path_openat+0xbe/0x6a0
[  218.279206]        [<ffffffff811cb7ca>] do_filp_open+0x3a/0x90
[  218.279718]        [<ffffffff811b8c1e>] do_sys_open+0x12e/0x210
[  218.280221]        [<ffffffff811b8d1e>] SyS_open+0x1e/0x20
[  218.280708]        [<ffffffff81732ce4>] tracesys+0xdd/0xe2
[  218.281184] 
-> #0 (&bdev->bd_mutex){+.+.+.}:
[  218.282109]        [<ffffffff810aec02>] __lock_acquire+0x1782/0x19f0
[  218.282622]        [<ffffffff810af623>] lock_acquire+0x93/0x1c0
[  218.283108]        [<ffffffff81725f67>] mutex_lock_nested+0x77/0x400
[  218.283582]        [<ffffffff8112f89f>] sysfs_blk_trace_attr_show+0x5f/0x1f0
[  218.284060]        [<ffffffff814b09f0>] dev_attr_show+0x20/0x60
[  218.284531]        [<ffffffff81240888>] sysfs_seq_show+0xc8/0x160
[  218.284997]        [<ffffffff811e2da2>] traverse.isra.6+0xf2/0x260
[  218.285468]        [<ffffffff811e3531>] seq_read+0x3e1/0x450
[  218.285929]        [<ffffffff811b9768>] vfs_read+0x98/0x170
[  218.286386]        [<ffffffff811ba412>] SyS_pread64+0x72/0xb0
[  218.286844]        [<ffffffff81732ce4>] tracesys+0xdd/0xe2
[  218.287299] 
other info that might help us debug this:

[  218.288691] Chain exists of:
  &bdev->bd_mutex --> &mm->mmap_sem --> &of->mutex

[  218.290060]  Possible unsafe locking scenario:

[  218.291013]        CPU0                    CPU1
[  218.291512]        ----                    ----
[  218.292003]   lock(&of->mutex);
[  218.292447]                                lock(&mm->mmap_sem);
[  218.292900]                                lock(&of->mutex);
[  218.293348]   lock(&bdev->bd_mutex);
[  218.293793] 
 *** DEADLOCK ***

[  218.295130] 3 locks held by trinity-child1/2100:
[  218.295615]  #0:  (&p->lock){+.+.+.}, at: [<ffffffff811e318d>] seq_read+0x3d/0x450
[  218.296068]  #1:  (&of->mutex){+.+.+.}, at: [<ffffffff8124083f>] sysfs_seq_show+0x7f/0x160
[  218.296555]  #2:  (s_active#113){.+.+.+}, at: [<ffffffff81240848>] sysfs_seq_show+0x88/0x160
[  218.297036] 
stack backtrace:
[  218.297952] CPU: 1 PID: 2100 Comm: trinity-child1 Not tainted 3.12.0+ #8 
[  218.299470]  ffffffff824ccc60 ffff8801bccabbd0 ffffffff8172017e ffffffff824c5c00
[  218.300013]  ffff8801bccabc10 ffffffff8171c51d ffff8801bccabc60 ffff8802411a0778
[  218.300571]  ffff8802411a0000 0000000000000002 0000000000000003 ffff8802411a07b0
[  218.301124] Call Trace:
[  218.301670]  [<ffffffff8172017e>] dump_stack+0x4e/0x7a
[  218.302227]  [<ffffffff8171c51d>] print_circular_bug+0x200/0x20f
[  218.302788]  [<ffffffff810aec02>] __lock_acquire+0x1782/0x19f0
[  218.303351]  [<ffffffff810af623>] lock_acquire+0x93/0x1c0
[  218.303916]  [<ffffffff8112f89f>] ? sysfs_blk_trace_attr_show+0x5f/0x1f0
[  218.304488]  [<ffffffff8112f89f>] ? sysfs_blk_trace_attr_show+0x5f/0x1f0
[  218.305051]  [<ffffffff81725f67>] mutex_lock_nested+0x77/0x400
[  218.305606]  [<ffffffff8112f89f>] ? sysfs_blk_trace_attr_show+0x5f/0x1f0
[  218.306165]  [<ffffffff8112f89f>] ? sysfs_blk_trace_attr_show+0x5f/0x1f0
[  218.306719]  [<ffffffff8112f89f>] sysfs_blk_trace_attr_show+0x5f/0x1f0
[  218.307268]  [<ffffffff814b09f0>] dev_attr_show+0x20/0x60
[  218.307837]  [<ffffffff8124048d>] ? sysfs_file_ops+0x5d/0x80
[  218.308389]  [<ffffffff81240888>] sysfs_seq_show+0xc8/0x160
[  218.308944]  [<ffffffff811e2da2>] traverse.isra.6+0xf2/0x260
[  218.309496]  [<ffffffff811e3531>] seq_read+0x3e1/0x450
[  218.310048]  [<ffffffff811b9768>] vfs_read+0x98/0x170
[  218.310601]  [<ffffffff811ba412>] SyS_pread64+0x72/0xb0
[  218.311157]  [<ffffffff81732ce4>] tracesys+0xdd/0xe2


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

Powered by blists - more mailing lists