On 27 September 2026 16:51:27 BST, Mathieu Desnoyers
<[email protected]> wrote:
>Hi Paul,
>
>This series applies on top of "hazptr: handle NULL address in
>hazptr_detach" you have in your rcu dev tree.
>
>This first patch addresses a race identified by Boqun Feng in the
>two-phase wildcard scheme.
>
>Patches 2-3 are prerequisites for using ptr_eq() in the 4th patch.
>Those were discussed at length in a prior version of hazard pointer
>patches.
>
>Patch 4 introduces a "try acquire" helper to allow the fast path
>to not rely on wildcards, while keeping the wildcard forward
>progress guarantees in the acquire slow path, used on fast path
>failure.
Hi, here is a hazptr perf test on powerpc
REAL kill_fasync(), ns per call, best of 3, 100k calls:
(stock = rwlock walk, conv = hazptr walk, same v3 tree ± the conversion)
shape stock conv delta
1 node, 1 walker 59 59 +0.0% (singleton: identical)
16 nodes, 1 walker 539 539 +0.0% (uncontended: identical)
16 nodes, 4 walkers 509 134 -73.7% ← rwlock readers contend
16 nodes, 8 walkers 313 113 -63.9% ← same list, 8 cpus
64 nodes, 1 walker 1979 2039 +3.0% (pure walk: hazptr tax)
64 nodes, 4 walkers 1914 509 -73.4%
64 nodes, 8 walkers 1015 382 -62.4%
Its SLOWER than rcu, but beats rwlock
SIGIO delivery, plain mode, 8 ptys 1 listener each (identical harness):
BASELINE (rwlock) 472/s
CONVERTED v4 (hazptr) 452/s ← singleton fast path: gap 12% → 4%
With a few changes, it was 12% slower than rcu before.
Do you want those changes?
My idea is, we find something that would put use to hazptr, here is what I
tried
diff --git a/fs/fcntl.c b/fs/fcntl.c
index c158f082f1da..bb04076ff6d9 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/pipe_fs_i.h>
+#include <linux/hazptr.h>
#include <linux/security.h>
#include <linux/ptrace.h>
#include <linux/signal.h>
@@ -1009,15 +1010,20 @@ int fasync_remove_entry(struct file *filp, struct
fasync_struct **fapp)
if (fa->fa_file != filp)
continue;
- write_lock_irq(&fa->fa_lock);
+ /*
+ * Make the file invisible to the walk before unlinking,
+ * then wait for any in-flight send_sigio() to be done with
+ * the node before freeing it. The walk holds a hazard
+ * pointer to this node, so it cannot already be freed.
+ */
fa->fa_file = NULL;
- write_unlock_irq(&fa->fa_lock);
-
*fp = fa->fa_next;
- kfree_rcu(fa, fa_rcu);
+ spin_unlock(&fasync_lock);
+ spin_unlock(&filp->f_lock);
+ hazptr_synchronize(fa);
+ fasync_free(fa);
filp->f_flags &= ~FASYNC;
- result = 1;
- break;
+ return 1;
}
spin_unlock(&fasync_lock);
spin_unlock(&filp->f_lock);
@@ -1056,13 +1062,10 @@ struct fasync_struct *fasync_insert_entry(int fd,
struct file *filp, struct fasy
if (fa->fa_file != filp)
continue;
- write_lock_irq(&fa->fa_lock);
- fa->fa_fd = fd;
- write_unlock_irq(&fa->fa_lock);
+ WRITE_ONCE(fa->fa_fd, fd);
goto out;
}
- rwlock_init(&new->fa_lock);
new->magic = FASYNC_MAGIC;
new->fa_file = filp;
new->fa_fd = fd;
@@ -1121,44 +1124,51 @@ EXPORT_SYMBOL(fasync_helper);
/*
* rcu_read_lock() is held
*/
-static void kill_fasync_rcu(struct fasync_struct *fa, int sig, int band)
+void kill_fasync(struct fasync_struct **fp, int sig, int band)
{
+ struct hazptr_ctx cur, nxt;
+ struct fasync_struct *fa;
+
+ /* First a quick test without locking: usually
+ * the list is empty.
+ */
+ fa = READ_ONCE(*fp);
+ if (!fa)
+ return;
+
+ /*
+ * Hand-over-hand with two ping-ponged contexts: the next node
+ * must be acquired before the current one is released, but a
+ * hazptr_ctx may only front one live slot at a time.
+ */
+ cur = (struct hazptr_ctx){ };
+ nxt = (struct hazptr_ctx){ };
+ fa = hazptr_acquire(&cur, (void * const *)fp);
while (fa) {
- struct fown_struct *fown;
- unsigned long flags;
+ struct fasync_struct *next;
if (fa->magic != FASYNC_MAGIC) {
printk(KERN_ERR "kill_fasync: bad magic number in "
"fasync_struct!\n");
- return;
+ break;
}
- read_lock_irqsave(&fa->fa_lock, flags);
+
if (fa->fa_file) {
- fown = file_f_owner(fa->fa_file);
- if (!fown)
- goto next;
- /* Don't send SIGURG to processes which have not set a
- queued signum: SIGURG has its own default signalling
- mechanism. */
- if (!(sig == SIGURG && fown->signum == 0))
+ struct fown_struct *fown = file_f_owner(fa->fa_file);
+
+ if (fown &&
+ /* Don't send SIGURG to processes which have not
set a
+ queued signum: SIGURG has its own default
signalling
+ mechanism. */
+ !(sig == SIGURG && fown->signum == 0))
send_sigio(fown, fa->fa_fd, band);
}
-next:
- read_unlock_irqrestore(&fa->fa_lock, flags);
- fa = rcu_dereference(fa->fa_next);
- }
-}
-
-void kill_fasync(struct fasync_struct **fp, int sig, int band)
-{
- /* First a quick test without locking: usually
- * the list is empty.
- */
- if (*fp) {
- rcu_read_lock();
- kill_fasync_rcu(rcu_dereference(*fp), sig, band);
- rcu_read_unlock();
+ next = hazptr_acquire(&nxt, (void * const *)&fa->fa_next);
+ hazptr_release(&cur, fa);
+ swap(cur, nxt);
+ fa = next;
}
+ hazptr_release(&cur, fa);
}
EXPORT_SYMBOL(kill_fasync);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f9d1e05e8ae6..852f1b8c00af 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1365,7 +1365,6 @@ static inline struct dentry *file_dentry(const struct
file *file)
}
struct fasync_struct {
- rwlock_t fa_lock;
int magic;
int fa_fd;
struct fasync_struct *fa_next; /* singly linked list */
Anything I did wrong? No?
>
>Thanks,
>
>Mathieu
>
>Mathieu Desnoyers (4):
> hazptr: Fix two-phase hazptr_synchronize race with detach
> compiler.h: Introduce ptr_eq() to preserve address dependency
> Documentation: RCU: Refer to ptr_eq()
> hazptr: Introduce "try acquire" fast path, fallback to overflow list
>
>Cc: Paul E. McKenney <[email protected]>
>Cc: Boqun Feng <[email protected]>
>Cc: Bradley Morgan <[email protected]>
>Cc: Gary Guo <[email protected]>
>Cc: <[email protected]>
>Cc: <[email protected]>
>
> Documentation/RCU/rcu_dereference.rst | 38 +++++++-
> include/linux/compiler.h | 63 ++++++++++++
> include/linux/hazptr.h | 47 +++++----
> kernel/hazptr.c | 135 +++++++++++++++-----------
> 4 files changed, 203 insertions(+), 80 deletions(-)
>
>
--- Thanks!
"I'm not a very positive person" - Linus torvalds
