[PATCH] kernfs: Improve lockdep annotation for files which implement mmap

[PATCH] kernfs: Improve lockdep annotation for files which implement mmap

[Bart Van Assche]

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