Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Joel Fernandes (Google)" <joel@joelfernandes.org>
To:     linux-kernel@vger.kernel.org
Cc:     "Joel Fernandes (Google)" <joel@joelfernandes.org>,
        Amir Goldstein <amir73il@gmail.com>, Jan Kara <jack@suse.cz>,
        linux-fsdevel@vger.kernel.org
Subject: [RFC] fs: Use slab constructor to initialize conn objects in fsnotify
Date:   Thu, 23 Apr 2020 00:40:50 -0400
Message-Id: <20200423044050.162093-1-joel@joelfernandes.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@joelfernandes.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