Re: sysfs_bin_mmap lockdep trace.

[Tejun Heo <tj@kernel.org>]

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.

Thanks.

-- 
tejun