From: Bart Van Assche <bvanassche@acm.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tejun Heo <tj@kernel.org>,
linux-kernel@vger.kernel.org,
Bart Van Assche <bvanassche@acm.org>,
Dave Jones <davej@redhat.com>
Subject: [PATCH] kernfs: Improve lockdep annotation for files which implement mmap
Date: Fri, 4 Oct 2019 09:11:24 -0700 [thread overview]
Message-ID: <20191004161124.111376-1-bvanassche@acm.org> (raw)
Instead of using one lockdep key for files that do not implement mmap
and another lockdep key for files that implement mmap, use a separate
lockdep key for each file that implements mmap.
This patch does not affect the size of struct kernfs_open_file if
CONFIG_LOCKDEP=n since in that case lock_class_key has size zero.
This patch is an improvement for commit 027a485d12e0 ("sysfs: use a
separate locking class for open files depending on mmap"; v3.13).
Cc: Dave Jones <davej@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
fs/kernfs/file.c | 34 +++++++++++++++++-----------------
include/linux/kernfs.h | 1 +
2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index e8c792b49616..a6980a8f9ffc 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -649,24 +649,17 @@ static int kernfs_fop_open(struct inode *inode, struct file *file)
goto err_out;
/*
- * The following is done to give a different lockdep key to
- * @of->mutex for files which implement mmap. This is a rather
- * crude way to avoid false positive lockdep warning around
- * mm->mmap_sem - mmap nests @of->mutex under mm->mmap_sem and
- * reading /sys/block/sda/trace/act_mask grabs sr_mutex, under
- * which mm->mmap_sem nests, while holding @of->mutex. As each
- * 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.
- *
- * Both paths of the branch look the same. They're supposed to
- * look that way and give @of->mutex different static lockdep keys.
+ * Assign a unique lockdep key to @of->mutex for files which implement
+ * mmap. This is necessary to avoid a false positive lockdep warning
+ * around mm->mmap_sem - mmap nests @of->mutex under mm->mmap_sem and
+ * reading /sys/block/sda/trace/act_mask grabs sr_mutex, under which
+ * mm->mmap_sem nests, while holding @of->mutex.
*/
- if (has_mmap)
- mutex_init(&of->mutex);
- else
- mutex_init(&of->mutex);
+ mutex_init(&of->mutex);
+ if (has_mmap) {
+ lockdep_register_key(&of->mutex_key);
+ lockdep_set_class(&of->mutex, &of->mutex_key);
+ }
of->kn = kn;
of->file = file;
@@ -734,6 +727,9 @@ static int kernfs_fop_open(struct inode *inode, struct file *file)
err_seq_release:
seq_release(inode, file);
err_free:
+ mutex_destroy(&of->mutex);
+ if (has_mmap)
+ lockdep_unregister_key(&of->mutex_key);
kfree(of->prealloc_buf);
kfree(of);
err_out:
@@ -769,6 +765,7 @@ static int kernfs_fop_release(struct inode *inode, struct file *filp)
{
struct kernfs_node *kn = inode->i_private;
struct kernfs_open_file *of = kernfs_of(filp);
+ const bool has_mmap = kn->attr.ops->mmap != NULL;
if (kn->flags & KERNFS_HAS_RELEASE) {
mutex_lock(&kernfs_open_file_mutex);
@@ -778,6 +775,9 @@ static int kernfs_fop_release(struct inode *inode, struct file *filp)
kernfs_put_open_node(kn, of);
seq_release(inode, filp);
+ mutex_destroy(&of->mutex);
+ if (has_mmap)
+ lockdep_unregister_key(&of->mutex_key);
kfree(of->prealloc_buf);
kfree(of);
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 936b61bd504e..6680fae5f0f6 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -204,6 +204,7 @@ struct kernfs_open_file {
void *priv;
/* private fields, do not use outside kernfs proper */
+ struct lock_class_key mutex_key;
struct mutex mutex;
struct mutex prealloc_mutex;
int event;
--
2.23.0.581.g78d2f28ef7-goog
reply other threads:[~2019-10-04 16:11 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20191004161124.111376-1-bvanassche@acm.org \
--to=bvanassche@acm.org \
--cc=davej@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tj@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;
as well as URLs for NNTP newsgroup(s).