From 7cb8f06ab022e7fc36bacbe65f654822147ec1aa Mon Sep 17 00:00:00 2001 From: Manfred Spraul Date: Fri, 27 Dec 2024 18:21:41 +0100 Subject: [PATCH] __wake_up_common_lock: Optimize for empty wake queues wake_up() must wake up every task that is in the wait queue. But: If there is concurrency between wake_up() and add_wait_queue(), then (as long as we are not violating the memory ordering rules) we can just claim that wake_up() happened "first" and add_wait_queue() happened afterwards. From memory ordering perspective: - If the wait_queue() is empty, then wake_up() just does spin_lock(); list_empty() spin_unlock(); - add_wait_queue()/prepare_to_wait() do all kind of operations, but they may become visible only when the spin_unlock_irqrestore() is done. Thus, instead of pairing the memory barrier to the spinlock, and thus writing to a potentially shared cacheline, we load-acquire the next pointer from the list. Risks and side effects: - The guaranteed memory barrier of wake_up() is reduced to load_acquire. Previously, there was always a spin_lock()/spin_unlock() pair. - prepare_to_wait() actually does two operations under spinlock: It adds current to the wait queue, and it calls set_current_state(). The comment above prepare_to_wait() is not clear to me, thus there might be further side effects. Only lightly tested. No benchmark test done. --- kernel/sched/wait.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c index 51e38f5f4701..10d02f652ab8 100644 --- a/kernel/sched/wait.c +++ b/kernel/sched/wait.c @@ -124,6 +124,23 @@ static int __wake_up_common_lock(struct wait_queue_head *wq_head, unsigned int m int __wake_up(struct wait_queue_head *wq_head, unsigned int mode, int nr_exclusive, void *key) { + if (list_empty(&wq_head->head)) { + struct list_head *pn; + + /* + * pairs with spin_unlock_irqrestore(&wq_head->lock); + * We actually do not need to acquire wq_head->lock, we just + * need to be sure that there is no prepare_to_wait() that + * completed on any CPU before __wake_up was called. + * Thus instead of load_acquiring the spinlock and dropping + * it again, we load_acquire the next list entry and check + * that the list is not empty. + */ + pn = smp_load_acquire(&wq_head->head.next); + + if(pn == &wq_head->head) + return 0; + } return __wake_up_common_lock(wq_head, mode, nr_exclusive, 0, key); } EXPORT_SYMBOL(__wake_up); -- 2.47.1