Skip to contentSkip to conversation

new locking primitive (pulled from fs_pin)

RFC In review Al Viro 2 participants 2 messages Apr 13, 2015

Subsystems (2)
VFS Locking primitives
lore
Share & tools
lore

Al Viro

author maintainer
#0 lore raw

RFC 1/1 new locking primitive (pulled from fs_pin)

New structure. Intended use: embed into an object that will always be freed with RCU delay.

Initialize with init_kill_once(&object->kill_once).

Use by grabbing rcu_read_lock(), finding the object, then

if (needs_killing(&object->kill_once)) {
	// do whatever actions you want, including
	// removal of references from the places
	// where they could be found
	finished_killing(&object->kill_once);
	// arrange for RCU-delayed freeing
}

If several threads attempt that, only the first one will see needs_killing() return true *and* everything else will wait in needs_killing() until the first one is past finished_killing(). Note that they might end up returning after object gets freed - needs_killing() itself is very careful about dereferencing and its caller MUST NOT touch the object after getting false from needs_killing().

needs_killing() must be called with rcu_read_lock() held and drops it in all cases.

This thing used to be the locking side of fs/fs_pin.c, but IMO it might make sense to separate it from fs_pin completely - it could be useful elsewhere...

Comments (and especially suggestions on better names) are welcome...

Signed-off-by Al Viro <[email protected]>

---

fs/fs_pin.c +2 -36
index b06c987..b124faf 100644
@@ -12,10 +12,7 @@ void pin_remove(struct fs_pin *pin)
 	hlist_del(&pin->m_list);
 	hlist_del(&pin->s_list);
 	spin_unlock(&pin_lock);
-	spin_lock_irq(&pin->wait.lock);
-	pin->done = 1;
-	wake_up_locked(&pin->wait);
-	spin_unlock_irq(&pin->wait.lock);
+	finished_killing(&pin->head);
 }
 
 void pin_insert_group(struct fs_pin *pin, struct vfsmount *m, struct hlist_head *p)

@@ -34,43 +31,12 @@ void pin_insert(struct fs_pin *pin, struct vfsmount *m)
 
 void pin_kill(struct fs_pin *p)
 {
-	wait_queue_t wait;
-
 	if (!p) {
 		rcu_read_unlock();
 		return;
 	}
-	init_wait(&wait);
-	spin_lock_irq(&p->wait.lock);
-	if (likely(!p->done)) {
-		p->done = -1;
-		spin_unlock_irq(&p->wait.lock);
-		rcu_read_unlock();
+	if (needs_killing(&p->head))
 		p->kill(p);
-		return;
-	}
-	if (p->done > 0) {
-		spin_unlock_irq(&p->wait.lock);
-		rcu_read_unlock();
-		return;
-	}
-	__add_wait_queue(&p->wait, &wait);
-	while (1) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		spin_unlock_irq(&p->wait.lock);
-		rcu_read_unlock();
-		schedule();
-		rcu_read_lock();
-		if (likely(list_empty(&wait.task_list)))
-			break;
-		/* OK, we know p couldn't have been freed yet */
-		spin_lock_irq(&p->wait.lock);
-		if (p->done > 0) {
-			spin_unlock_irq(&p->wait.lock);
-			break;
-		}
-	}
-	rcu_read_unlock();
 }
 
 void mnt_pin_kill(struct mount *m)
include/linux/fs_pin.h +3 -4
index 9dc4e03..f65daad 100644
@@ -1,8 +1,7 @@
-#include <linux/wait.h>
+#include <linux/kill_once.h>
 
 struct fs_pin {
-	wait_queue_head_t	wait;
-	int			done;
+	struct kill_once	head;
 	struct hlist_node	s_list;
 	struct hlist_node	m_list;
 	void (*kill)(struct fs_pin *);

@@ -12,7 +11,7 @@ struct vfsmount;
 
 static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *))
 {
-	init_waitqueue_head(&p->wait);
+	init_kill_once(&p->head);
 	p->kill = kill;
 }
include/linux/kill_once.h new file +28 -0
new file mode 100644
index 0000000..03a3717
@@ -0,0 +1,28 @@
+#include <linux/wait.h>
+
+/*
+Intended use:
+	rcu_read_lock();
+	p = <....>
+	if (needs_killing(p)) {
+		kill it
+		finished_killing(p);
+		arrange for rcu-delayed freeing
+	} else {
+		we are guaranteed that it is an ex-parrot
+	}
+*/
+
+struct kill_once {
+	wait_queue_head_t	wait;
+	int			done;
+};
+
+static inline void init_kill_once(struct kill_once *p)
+{
+	init_waitqueue_head(&p->wait);
+	p->done = 0;
+}
+
+bool needs_killing(struct kill_once *);
+void finished_killing(struct kill_once *);
kernel/locking/Makefile +1 -1
index de7a416..c404207 100644
@@ -1,5 +1,5 @@
 
-obj-y += mutex.o semaphore.o rwsem.o
+obj-y += mutex.o semaphore.o rwsem.o kill_once.o
 
 ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_lockdep.o = $(CC_FLAGS_FTRACE)
kernel/locking/kill_once.c new file +59 -0

59 lines not on this page · raw message

#1 lore raw
Quoted 6 lines · Al Viro

This thing used to be the locking side of fs/fs_pin.c, but IMO it might make sense to separate it from fs_pin completely - it could be useful elsewhere...

Comments (and especially suggestions on better names) are welcome...

I don't mind abstracting it out, but I would prefer that it stay local to fs_pin.c until there's a clear case for using it anywhere else.

Do you really have any usage cases that are interesting? Because quite frankly, I'm not a fan of the nasty fs-pin code, and I would *not* want the ideas in there to bleed out anywhere else.

So my gut feel is that it's a good thing to try to abstract out that mess, but not a good idea to spread it out.

(I do realize that without the cross-file movement, the patch basically just boils down to abstracting out that "finished_killing()" into a helper function and little else)

Linus