mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Dave Jones <davej@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	gregkh@linuxfoundation.org
Subject: [PATCH driver-core-linus] sysfs: give different locking key to regular and bin files
Date: Wed, 4 Dec 2013 09:20:40 -0500	[thread overview]
Message-ID: <20131204142040.GK3158@htj.dyndns.org> (raw)
In-Reply-To: <20131204141345.GJ3158@htj.dyndns.org>

Dave, can you please test this one too?  Greg, once this turns out to
be okay, I'll send you a merged branch which pulls in
driver-core-linus + this patch into driver-core-next which will surely
generate conflict.

Thanks.
----- 8< ------
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.

Fix it by restoring the original behavior fully by differentiating
lockdep key by whether the file is regular or bin, instead of the
existence of mmap.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Dave Jones <davej@redhat.com>
Link: http://lkml.kernel.org/g/20131203184324.GA11320@redhat.com
---
 fs/sysfs/file.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index b94f936..35e7d08 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -609,7 +609,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
 	struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
 	struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
 	struct sysfs_open_file *of;
-	bool has_read, has_write, has_mmap;
+	bool has_read, has_write;
 	int error = -EACCES;
 
 	/* need attr_sd for attr and ops, its parent for kobj */
@@ -621,7 +621,6 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
 
 		has_read = battr->read || battr->mmap;
 		has_write = battr->write || battr->mmap;
-		has_mmap = battr->mmap;
 	} else {
 		const struct sysfs_ops *ops = sysfs_file_ops(attr_sd);
 
@@ -633,7 +632,6 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
 
 		has_read = ops->show;
 		has_write = ops->store;
-		has_mmap = false;
 	}
 
 	/* check perms and supported operations */
@@ -661,9 +659,9 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
 	 * open file has a separate mutex, it's okay as long as those don't
 	 * happen on the same file.  At this point, we can't easily give
 	 * each file a separate locking class.  Let's differentiate on
-	 * whether the file has mmap or not for now.
+	 * whether the file is bin or not for now.
 	 */
-	if (has_mmap)
+	if (sysfs_is_bin(attr_sd))
 		mutex_init(&of->mutex);
 	else
 		mutex_init(&of->mutex);

  reply	other threads:[~2013-12-04 14:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20131128051223.45739660885@gitolite.kernel.org>
2013-12-03 18:43 ` sysfs: use a separate locking class for open files depending on mmap Dave Jones
2013-12-03 21:10   ` Tejun Heo
2013-12-03 21:15     ` Dave Jones
2013-12-03 21:36       ` Tejun Heo
2013-12-03 22:15         ` Tejun Heo
2013-12-04  4:43           ` Dave Jones
2013-12-04 14:06             ` [PATCH driver-core-linus] sysfs: bail early from sysfs_bin_mmap() to avoid spurious lockdep warning Tejun Heo
2013-12-04 14:13               ` Tejun Heo
2013-12-04 14:20                 ` Tejun Heo [this message]
2013-12-05 19:44                   ` [PATCH driver-core-linus] sysfs: give different locking key to regular and bin files Greg KH
2013-12-05 19:52                     ` Dave Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131204142040.GK3158@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=davej@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®