* [PATCH] sched/wait: avoid modifying entry on failed priority registration
@ 2026-10-11 1:50 Hui Su
2026-10-12 13:32 ` Sean Christopherson
0 siblings, 1 reply; 2+ messages in thread
From: Hui Su @ 2026-10-11 1:50 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, Sean Christopherson,
linux-kernel
add_wait_queue_priority_exclusive() sets WQ_FLAG_EXCLUSIVE and
WQ_FLAG_PRIORITY before checking whether another priority waiter is
already present. Thus, when registration fails with -EBUSY, the entry
is left unqueued but with modified flags.
There is no need to modify the entry before the failure check. Move the
flag update after the check so that a rejected registration does not
unnecessarily alter the entry.
Signed-off-by: Hui Su <sh_def@163.com>
---
Testing:
- Private x86_64/QEMU differential oracle: the old path modified the entry
flags on -EBUSY; the patched path preserved them.
---
kernel/sched/wait.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index d033f600f48c..93b353fbd0c9 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -54,14 +54,13 @@ int add_wait_queue_priority_exclusive(struct wait_queue_head *wq_head,
{
struct list_head *head = &wq_head->head;
- wq_entry->flags |= WQ_FLAG_EXCLUSIVE | WQ_FLAG_PRIORITY;
-
guard(spinlock_irqsave)(&wq_head->lock);
if (!list_empty(head) &&
(list_first_entry(head, typeof(*wq_entry), entry)->flags & WQ_FLAG_PRIORITY))
return -EBUSY;
+ wq_entry->flags |= WQ_FLAG_EXCLUSIVE | WQ_FLAG_PRIORITY;
list_add(&wq_entry->entry, head);
return 0;
}
base-commit: 3857c2fe5449541e24afc5efdb0f81a8a8f9a3a0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] sched/wait: avoid modifying entry on failed priority registration
2026-10-11 1:50 [PATCH] sched/wait: avoid modifying entry on failed priority registration Hui Su
@ 2026-10-12 13:32 ` Sean Christopherson
0 siblings, 0 replies; 2+ messages in thread
From: Sean Christopherson @ 2026-10-12 13:32 UTC (permalink / raw)
To: Hui Su
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, linux-kernel
On Sun, Oct 11, 2026, Hui Su wrote:
> add_wait_queue_priority_exclusive() sets WQ_FLAG_EXCLUSIVE and
> WQ_FLAG_PRIORITY before checking whether another priority waiter is
> already present. Thus, when registration fails with -EBUSY, the entry
> is left unqueued but with modified flags.
>
> There is no need to modify the entry before the failure check. Move the
> flag update after the check so that a rejected registration does not
> unnecessarily alter the entry.
In case anyone is concerned about this being a meaningful bug, kvm_irqfd_register()
is the only user of add_wait_queue_priority_exclusive() and KVM never touches its
entry if registration fails. I.e. this is completely benign in the current code
base.
> Signed-off-by: Hui Su <sh_def@163.com>
>
> ---
> Testing:
> - Private x86_64/QEMU differential oracle: the old path modified the entry
> flags on -EBUSY; the patched path preserved them.
> ---
> kernel/sched/wait.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
> index d033f600f48c..93b353fbd0c9 100644
> --- a/kernel/sched/wait.c
> +++ b/kernel/sched/wait.c
> @@ -54,14 +54,13 @@ int add_wait_queue_priority_exclusive(struct wait_queue_head *wq_head,
> {
> struct list_head *head = &wq_head->head;
>
> - wq_entry->flags |= WQ_FLAG_EXCLUSIVE | WQ_FLAG_PRIORITY;
> -
> guard(spinlock_irqsave)(&wq_head->lock);
>
> if (!list_empty(head) &&
> (list_first_entry(head, typeof(*wq_entry), entry)->flags & WQ_FLAG_PRIORITY))
> return -EBUSY;
>
> + wq_entry->flags |= WQ_FLAG_EXCLUSIVE | WQ_FLAG_PRIORITY;
AFAICT, nothing peeks at wait queue entries outside of the spinlock (which is
stating the obvious to some extent since there's no RCU in sight), so we're not
relying on the above spinlock acquisition to order the flags update before the
list_add().
Reviewed-by: Sean Christopherson <seanjc@google.com>
> list_add(&wq_entry->entry, head);
> return 0;
> }
>
> base-commit: 3857c2fe5449541e24afc5efdb0f81a8a8f9a3a0
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-12 13:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-11 1:50 [PATCH] sched/wait: avoid modifying entry on failed priority registration Hui Su
2026-10-12 13:32 ` Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®