Re: [PATCH driver-core-linus] sysfs: bail early fromsysfs_bin_mmap() to avoid spurious lockdep warning

From: Tejun Heo
Date: Wed Dec 04 2013 - 09:13:54 EST


On Wed, Dec 04, 2013 at 09:06:39AM -0500, Tejun Heo wrote:
> 027a485d12e0 ("sysfs: use a separate locking class for open files
> depending on mmap") assigned different lockdep key to
> sysfs_open_file->mutex depending on whether the file implements mmap
> or not in an attempt to avoid spurious lockdep warning caused by
> merging of regular and bin file paths.
>
> While this restored some of the original behavior of using different
> locks (at least lockdep is concerned) for the different clases of
> files. The restoration wasn't full because now the lockdep key
> assignment depends on whether the file has mmap or not instead of
> whether it's a regular file or not.
>
> This means that bin files which don't implement mmap will get assigned
> the same lockdep class as regular files. This is problematic because
> file_operations for bin files still implements the mmap file operation
> and checking whether the sysfs file actually implements mmap happens
> in the file operation after grabbing @sysfs_open_file->mutex. We
> still end up adding locking dependency from mmap locking to
> sysfs_open_file->mutex to the regular file mutex which triggers
> spurious circular locking warning.
>
> This can be fixed by either giving sysfs_open_file->mutex different
> lockdep keys depending on whether the file is regular or bin instead
> of whether mmap exists or not, or avoiding grabbing
> sysfs_open_file->mutex from sysfs_bin_mmap() if mmap is not actually
> implemented. While the former is simpler for driver-core-linus,
> driver-core-next already has SYSFS_FLAG_HAS_MMAP in place to implement
> the latter and doesn't have inherent distinction between regular and
> bin files. This patch implements the latter so that the fix is more
> conducive to driver-core-next.
>
> Because anything beyond sysfs_open_file->sd can't be dereferenced
> without locking the open file, cache whether mmap is implemented or
> not in sysfs_open_file->sd->s_flags and update sysfs_bin_mmap() test
> the flag and bail without grabbing the mutex if not implemented.
>
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> Reported-by: Dave Jones <davej@xxxxxxxxxx>
> Tested-by: Dave Jones <davej@xxxxxxxxxx>
> Link: http://lkml.kernel.org/g/20131203184324.GA11320@xxxxxxxxxx

Please forget about this. This is actually a lot harder to translate
to kernfs. I'll prepare another patch. Sorry about the noise.

Thanks.

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