linux-kernel - [RFC] fs: Use slab constructor to initialize conn objects in fsnotify

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 23 Apr 2020 00:40:50 -0400
From:   "Joel Fernandes (Google)" <joel@...lfernandes.org>
To:     linux-kernel@...r.kernel.org
Cc:     "Joel Fernandes (Google)" <joel@...lfernandes.org>,
        Amir Goldstein <amir73il@...il.com>, Jan Kara <jack@...e.cz>,
        linux-fsdevel@...r.kernel.org
Subject: [RFC] fs: Use slab constructor to initialize conn objects in fsnotify

While reading the famous slab paper [1], I noticed that the conn->lock
spinlock and conn->list hlist in fsnotify code is being initialized
during every object allocation. This seems a good fit for the
constructor within the slab to take advantage of the slab design. Move
the initializtion to that.

       spin_lock_init(&conn->lock);
       INIT_HLIST_HEAD(&conn->list);

[1] https://pdfs.semanticscholar.org/1acc/3a14da69dd240f2fbc11d00e09610263bdbd.pdf

Signed-off-by: Joel Fernandes (Google) <joel@...lfernandes.org>
---
I've only build-tested. I am not very familiar with this code, so I
kindly request the maintainers/reviewers to take a look and see if it
makes sense. From what I see, the ->list is always empty during
allocation. The spinlock allocation also seems unnecesary.


 fs/notify/fsnotify.c | 15 +++++++++++++--
 fs/notify/mark.c     |  2 --
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 46f2255800091..55ed450da3295 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -385,6 +385,14 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 }
 EXPORT_SYMBOL_GPL(fsnotify);
 
+static void fsnotify_ctor(void *addr)
+{
+	struct fsnotify_mark_connector *conn = addr;
+
+	spin_lock_init(&conn->lock);
+	INIT_HLIST_HEAD(&conn->list);
+}
+
 static __init int fsnotify_init(void)
 {
 	int ret;
@@ -395,8 +403,11 @@ static __init int fsnotify_init(void)
 	if (ret)
 		panic("initializing fsnotify_mark_srcu");
 
-	fsnotify_mark_connector_cachep = KMEM_CACHE(fsnotify_mark_connector,
-						    SLAB_PANIC);
+	fsnotify_mark_connector_cachep =
+		kmem_cache_create("fsnotify_mark_connector",
+				  sizeof(struct fsnotify_mark_connector),
+				  __alignof__(struct fsnotify_mark_connector),
+				  SLAB_PANIC, fsnotify_ctor);
 
 	return 0;
 }
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 1d96216dffd19..d388e7f45e2ea 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -479,8 +479,6 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
 	conn = kmem_cache_alloc(fsnotify_mark_connector_cachep, GFP_KERNEL);
 	if (!conn)
 		return -ENOMEM;
-	spin_lock_init(&conn->lock);
-	INIT_HLIST_HEAD(&conn->list);
 	conn->type = type;
 	conn->obj = connp;
 	/* Cache fsid of filesystem containing the object */
-- 
2.26.1.301.g55bc3eb7cb9-goog