lkml.org 
[lkml]   [2019]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] kernfs: Improve lockdep annotation for files which implement mmap
Date
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
\
 
 \ /
  Last update: 2019-10-04 18:13    [W:0.051 / U:5.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site