All of lore.kernel.org
 help / color / mirror / Atom feed
From: Imran Khan <imran.f.khan@oracle.com>
To: tj@kernel.org, gregkh@linuxfoundation.org, viro@zeniv.linux.org.uk
Cc: m.szyprowski@samsung.com, nathan@kernel.org, michael@walle.cc,
	robh@kernel.org, linux-serial@vger.kernel.org,
	linux-kernel@vger.kernel.org, guillaume.tucker@collabora.com,
	pmladek@suse.com
Subject: [PATCH] Revert "kernfs: Change kernfs_notify_list to llist."
Date: Wed,  6 Jul 2022 06:10:26 +1000	[thread overview]
Message-ID: <20220705201026.2487665-1-imran.f.khan@oracle.com> (raw)

This reverts commit b8f35fa1188b84035c59d4842826c4e93a1b1c9f.

This is causing regression due to same kernfs_node getting
added multiple times in kernfs_notify_list so revert it until
safe way of using llist in this context is found.

Reported-by: Nathan Chancellor <nathan@kernel.org>
Reported-by: Michael Walle <michael@walle.cc>
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>

Signed-off-by: Imran Khan <imran.f.khan@oracle.com>
---
 fs/kernfs/file.c       | 47 ++++++++++++++++++++++++------------------
 include/linux/kernfs.h |  2 +-
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index bb933221b4bae..baff4b1d40c76 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -25,16 +25,18 @@ struct kernfs_open_node {
 	struct list_head	files; /* goes through kernfs_open_file.list */
 };
 
-/**
- * attribute_to_node - get kernfs_node object corresponding to a kernfs attribute
- * @ptr:	&struct kernfs_elem_attr
- * @type:	struct kernfs_node
- * @member:	name of member (i.e attr)
+/*
+ * kernfs_notify() may be called from any context and bounces notifications
+ * through a work item.  To minimize space overhead in kernfs_node, the
+ * pending queue is implemented as a singly linked list of kernfs_nodes.
+ * The list is terminated with the self pointer so that whether a
+ * kernfs_node is on the list or not can be determined by testing the next
+ * pointer for NULL.
  */
-#define attribute_to_node(ptr, type, member)	\
-	container_of(ptr, type, member)
+#define KERNFS_NOTIFY_EOL			((void *)&kernfs_notify_list)
 
-static LLIST_HEAD(kernfs_notify_list);
+static DEFINE_SPINLOCK(kernfs_notify_lock);
+static struct kernfs_node *kernfs_notify_list = KERNFS_NOTIFY_EOL;
 
 static inline struct mutex *kernfs_open_file_mutex_ptr(struct kernfs_node *kn)
 {
@@ -909,16 +911,18 @@ static void kernfs_notify_workfn(struct work_struct *work)
 	struct kernfs_node *kn;
 	struct kernfs_super_info *info;
 	struct kernfs_root *root;
-	struct llist_node *free;
-	struct kernfs_elem_attr *attr;
 repeat:
 	/* pop one off the notify_list */
-	free = llist_del_first(&kernfs_notify_list);
-	if (free == NULL)
+	spin_lock_irq(&kernfs_notify_lock);
+	kn = kernfs_notify_list;
+	if (kn == KERNFS_NOTIFY_EOL) {
+		spin_unlock_irq(&kernfs_notify_lock);
 		return;
+	}
+	kernfs_notify_list = kn->attr.notify_next;
+	kn->attr.notify_next = NULL;
+	spin_unlock_irq(&kernfs_notify_lock);
 
-	attr = llist_entry(free, struct kernfs_elem_attr, notify_next);
-	kn = attribute_to_node(attr, struct kernfs_node, attr);
 	root = kernfs_root(kn);
 	/* kick fsnotify */
 	down_write(&root->kernfs_rwsem);
@@ -974,14 +978,12 @@ static void kernfs_notify_workfn(struct work_struct *work)
 void kernfs_notify(struct kernfs_node *kn)
 {
 	static DECLARE_WORK(kernfs_notify_work, kernfs_notify_workfn);
+	unsigned long flags;
 	struct kernfs_open_node *on;
 
 	if (WARN_ON(kernfs_type(kn) != KERNFS_FILE))
 		return;
 
-	/* Because we are using llist for kernfs_notify_list */
-	WARN_ON_ONCE(in_nmi());
-
 	/* kick poll immediately */
 	rcu_read_lock();
 	on = rcu_dereference(kn->attr.open);
@@ -992,9 +994,14 @@ void kernfs_notify(struct kernfs_node *kn)
 	rcu_read_unlock();
 
 	/* schedule work to kick fsnotify */
-	kernfs_get(kn);
-	llist_add(&kn->attr.notify_next, &kernfs_notify_list);
-	schedule_work(&kernfs_notify_work);
+	spin_lock_irqsave(&kernfs_notify_lock, flags);
+	if (!kn->attr.notify_next) {
+		kernfs_get(kn);
+		kn->attr.notify_next = kernfs_notify_list;
+		kernfs_notify_list = kn;
+		schedule_work(&kernfs_notify_work);
+	}
+	spin_unlock_irqrestore(&kernfs_notify_lock, flags);
 }
 EXPORT_SYMBOL_GPL(kernfs_notify);
 
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 13e703f615f79..367044d7708c6 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -173,7 +173,7 @@ struct kernfs_elem_attr {
 	const struct kernfs_ops	*ops;
 	struct kernfs_open_node __rcu	*open;
 	loff_t			size;
-	struct llist_node	notify_next;	/* for kernfs_notify() */
+	struct kernfs_node	*notify_next;	/* for kernfs_notify() */
 };
 
 /*

base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
-- 
2.30.2


             reply	other threads:[~2022-07-05 20:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-05 20:10 Imran Khan [this message]
2022-07-08 16:02 ` [PATCH] Revert "kernfs: Change kernfs_notify_list to llist." Andy Shevchenko

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=20220705201026.2487665-1-imran.f.khan@oracle.com \
    --to=imran.f.khan@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=guillaume.tucker@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=michael@walle.cc \
    --cc=nathan@kernel.org \
    --cc=pmladek@suse.com \
    --cc=robh@kernel.org \
    --cc=tj@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.