This patchset contains changes that were suggested as part of review of [1].
I am sending these changes as a separate patchset because [1] focusses on
replacing some global locks with hashed ones while these changes are applicable
irrespective of whether we stick with global locks or endup using hashed locks
eventually and hence these changes can be reviewed independent of [1].
[1] https://lore.kernel.org/lkml/[email protected]/
Imran Khan (2):
kernfs: Remove reference counting for kernfs_open_node.
kernfs: make ->attr.open RCU protected.
fs/kernfs/file.c | 92 +++++++++++++++++-------------------------
include/linux/kernfs.h | 2 +-
2 files changed, 39 insertions(+), 55 deletions(-)
base-commit: dd315b5800612e6913343524aa9b993f9a8bb0cf
--
2.30.2
After removal of kernfs_open_node->refcnt in the previous patch,
kernfs_open_node_lock can be removed as well by making ->attr.open
RCU protected. kernfs_put_open_node can delegate freeing to ->attr.open
to RCU and other readers of ->attr.open can do so under rcu_read_(un)lock.
So make ->attr.open RCU protected and remove global kernfs_open_node_lock.
Suggested by: Al Viro <[email protected]>
Signed-off-by: Imran Khan <[email protected]>
---
fs/kernfs/file.c | 77 ++++++++++++++++++------------------------
include/linux/kernfs.h | 2 +-
2 files changed, 34 insertions(+), 45 deletions(-)
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index aea6968c979e..b6d50769171b 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -23,16 +23,16 @@
* for each kernfs_node with one or more open files.
*
* kernfs_node->attr.open points to kernfs_open_node. attr.open is
- * protected by kernfs_open_node_lock.
+ * RCU protected.
*
* filp->private_data points to seq_file whose ->private points to
* kernfs_open_file. kernfs_open_files are chained at
* kernfs_open_node->files, which is protected by kernfs_open_file_mutex.
*/
-static DEFINE_SPINLOCK(kernfs_open_node_lock);
static DEFINE_MUTEX(kernfs_open_file_mutex);
struct kernfs_open_node {
+ struct rcu_head rcu_head;
atomic_t event;
wait_queue_head_t poll;
struct list_head files; /* goes through kernfs_open_file.list */
@@ -519,36 +519,32 @@ static int kernfs_get_open_node(struct kernfs_node *kn,
{
struct kernfs_open_node *on, *new_on = NULL;
- retry:
mutex_lock(&kernfs_open_file_mutex);
- spin_lock_irq(&kernfs_open_node_lock);
-
- if (!kn->attr.open && new_on) {
- kn->attr.open = new_on;
- new_on = NULL;
- }
-
- on = kn->attr.open;
- if (on)
- list_add_tail(&of->list, &on->files);
-
- spin_unlock_irq(&kernfs_open_node_lock);
- mutex_unlock(&kernfs_open_file_mutex);
+ rcu_read_lock();
+ on = rcu_dereference(kn->attr.open);
if (on) {
- kfree(new_on);
+ list_add_tail(&of->list, &on->files);
+ rcu_read_unlock();
+ mutex_unlock(&kernfs_open_file_mutex);
return 0;
+ } else {
+ rcu_read_unlock();
+ /* not there, initialize a new one and retry */
+ new_on = kmalloc(sizeof(*new_on), GFP_KERNEL);
+ if (!new_on) {
+ mutex_unlock(&kernfs_open_file_mutex);
+ return -ENOMEM;
+ }
+ atomic_set(&new_on->event, 1);
+ init_waitqueue_head(&new_on->poll);
+ INIT_LIST_HEAD(&new_on->files);
+ list_add_tail(&of->list, &new_on->files);
+ rcu_assign_pointer(kn->attr.open, new_on);
}
+ mutex_unlock(&kernfs_open_file_mutex);
- /* not there, initialize a new one and retry */
- new_on = kmalloc(sizeof(*new_on), GFP_KERNEL);
- if (!new_on)
- return -ENOMEM;
-
- atomic_set(&new_on->event, 1);
- init_waitqueue_head(&new_on->poll);
- INIT_LIST_HEAD(&new_on->files);
- goto retry;
+ return 0;
}
/**
@@ -566,24 +562,18 @@ static int kernfs_get_open_node(struct kernfs_node *kn,
static void kernfs_put_open_node(struct kernfs_node *kn,
struct kernfs_open_file *of)
{
- struct kernfs_open_node *on = kn->attr.open;
- unsigned long flags;
+ struct kernfs_open_node *on = rcu_dereference_raw(kn->attr.open);
mutex_lock(&kernfs_open_file_mutex);
- spin_lock_irqsave(&kernfs_open_node_lock, flags);
if (of)
list_del(&of->list);
- if (list_empty(&on->files))
- kn->attr.open = NULL;
- else
- on = NULL;
-
- spin_unlock_irqrestore(&kernfs_open_node_lock, flags);
+ if (list_empty(&on->files)) {
+ rcu_assign_pointer(kn->attr.open, NULL);
+ kfree_rcu(on, rcu_head);
+ }
mutex_unlock(&kernfs_open_file_mutex);
-
- kfree(on);
}
static int kernfs_fop_open(struct inode *inode, struct file *file)
@@ -765,12 +755,12 @@ void kernfs_drain_open_files(struct kernfs_node *kn)
if (!(kn->flags & (KERNFS_HAS_MMAP | KERNFS_HAS_RELEASE)))
return;
- on = kn->attr.open;
+ on = rcu_dereference_raw(kn->attr.open);
if (!on)
return;
mutex_lock(&kernfs_open_file_mutex);
- if (!kn->attr.open) {
+ if (!rcu_dereference_raw(kn->attr.open)) {
mutex_unlock(&kernfs_open_file_mutex);
return;
}
@@ -805,7 +795,7 @@ void kernfs_drain_open_files(struct kernfs_node *kn)
__poll_t kernfs_generic_poll(struct kernfs_open_file *of, poll_table *wait)
{
struct kernfs_node *kn = kernfs_dentry_node(of->file->f_path.dentry);
- struct kernfs_open_node *on = kn->attr.open;
+ struct kernfs_open_node *on = rcu_dereference_raw(kn->attr.open);
poll_wait(of->file, &on->poll, wait);
@@ -912,14 +902,13 @@ void kernfs_notify(struct kernfs_node *kn)
return;
/* kick poll immediately */
- spin_lock_irqsave(&kernfs_open_node_lock, flags);
- on = kn->attr.open;
+ rcu_read_lock();
+ on = rcu_dereference(kn->attr.open);
if (on) {
atomic_inc(&on->event);
wake_up_interruptible(&on->poll);
}
- spin_unlock_irqrestore(&kernfs_open_node_lock, flags);
-
+ rcu_read_unlock();
/* schedule work to kick fsnotify */
spin_lock_irqsave(&kernfs_notify_lock, flags);
if (!kn->attr.notify_next) {
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index e2ae15a6225e..13f54f078a52 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -114,7 +114,7 @@ struct kernfs_elem_symlink {
struct kernfs_elem_attr {
const struct kernfs_ops *ops;
- struct kernfs_open_node *open;
+ struct kernfs_open_node __rcu *open;
loff_t size;
struct kernfs_node *notify_next; /* for kernfs_notify() */
};
--
2.30.2