From: Shrikanth Hegde <sshegde@linux.ibm.com>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
Valentin Schneider <vschneid@redhat.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched/wait: Use guard() for wq_head->lock
Date: Tue, 15 Apr 2025 23:43:08 +0530 [thread overview]
Message-ID: <f9f8144c-7666-4675-a17b-b31bed2db6c5@linux.ibm.com> (raw)
In-Reply-To: <20250402183843.15540-1-kprateek.nayak@amd.com>
On 4/3/25 00:08, K Prateek Nayak wrote:
> Using guard(spinlock_irqsave) for "wq_head->lock" helps eliminate the
> local "flags" variable, the local "ret" variable in some cases, and
> helps simplify the flow in prepare_to_wait_event().
>
flags variable becomes implicit now, only ret is eliminated in some cases.
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
> kernel/sched/wait.c | 69 ++++++++++++++++-----------------------------
> 1 file changed, 25 insertions(+), 44 deletions(-)
>
> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
> index 51e38f5f4701..2cf7687e00e0 100644
> --- a/kernel/sched/wait.c
> +++ b/kernel/sched/wait.c
> @@ -16,44 +16,35 @@ EXPORT_SYMBOL(__init_waitqueue_head);
>
> void add_wait_queue(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry)
> {
> - unsigned long flags;
> -
> wq_entry->flags &= ~WQ_FLAG_EXCLUSIVE;
> - spin_lock_irqsave(&wq_head->lock, flags);
> +
> + guard(spinlock_irqsave)(&wq_head->lock);
> __add_wait_queue(wq_head, wq_entry);
> - spin_unlock_irqrestore(&wq_head->lock, flags);
> }
> EXPORT_SYMBOL(add_wait_queue);
>
> void add_wait_queue_exclusive(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry)
> {
> - unsigned long flags;
> -
> wq_entry->flags |= WQ_FLAG_EXCLUSIVE;
> - spin_lock_irqsave(&wq_head->lock, flags);
> +
> + guard(spinlock_irqsave)(&wq_head->lock);
> __add_wait_queue_entry_tail(wq_head, wq_entry);
> - spin_unlock_irqrestore(&wq_head->lock, flags);
> }
> EXPORT_SYMBOL(add_wait_queue_exclusive);
>
> void add_wait_queue_priority(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry)
> {
> - unsigned long flags;
> -
> wq_entry->flags |= WQ_FLAG_EXCLUSIVE | WQ_FLAG_PRIORITY;
> - spin_lock_irqsave(&wq_head->lock, flags);
> +
> + guard(spinlock_irqsave)(&wq_head->lock);
> __add_wait_queue(wq_head, wq_entry);
> - spin_unlock_irqrestore(&wq_head->lock, flags);
> }
> EXPORT_SYMBOL_GPL(add_wait_queue_priority);
>
> void remove_wait_queue(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry)
> {
> - unsigned long flags;
> -
> - spin_lock_irqsave(&wq_head->lock, flags);
> + guard(spinlock_irqsave)(&wq_head->lock);
> __remove_wait_queue(wq_head, wq_entry);
> - spin_unlock_irqrestore(&wq_head->lock, flags);
> }
> EXPORT_SYMBOL(remove_wait_queue);
>
> @@ -99,13 +90,11 @@ static int __wake_up_common(struct wait_queue_head *wq_head, unsigned int mode,
> static int __wake_up_common_lock(struct wait_queue_head *wq_head, unsigned int mode,
> int nr_exclusive, int wake_flags, void *key)
> {
> - unsigned long flags;
> int remaining;
>
> - spin_lock_irqsave(&wq_head->lock, flags);
> + guard(spinlock_irqsave)(&wq_head->lock);
> remaining = __wake_up_common(wq_head, mode, nr_exclusive, wake_flags,
> key);
> - spin_unlock_irqrestore(&wq_head->lock, flags);
>
> return nr_exclusive - remaining;
> }
> @@ -228,14 +217,12 @@ void __wake_up_pollfree(struct wait_queue_head *wq_head)
> void
> prepare_to_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry, int state)
> {
> - unsigned long flags;
> -
> wq_entry->flags &= ~WQ_FLAG_EXCLUSIVE;
> - spin_lock_irqsave(&wq_head->lock, flags);
> +
> + guard(spinlock_irqsave)(&wq_head->lock);
> if (list_empty(&wq_entry->entry))
> __add_wait_queue(wq_head, wq_entry);
> set_current_state(state);
> - spin_unlock_irqrestore(&wq_head->lock, flags);
> }
> EXPORT_SYMBOL(prepare_to_wait);
>
> @@ -243,17 +230,17 @@ EXPORT_SYMBOL(prepare_to_wait);
> bool
> prepare_to_wait_exclusive(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry, int state)
> {
> - unsigned long flags;
> bool was_empty = false;
>
> wq_entry->flags |= WQ_FLAG_EXCLUSIVE;
> - spin_lock_irqsave(&wq_head->lock, flags);
> +
> + guard(spinlock_irqsave)(&wq_head->lock);
> if (list_empty(&wq_entry->entry)) {
> was_empty = list_empty(&wq_head->head);
> __add_wait_queue_entry_tail(wq_head, wq_entry);
> }
> set_current_state(state);
> - spin_unlock_irqrestore(&wq_head->lock, flags);
> +
> return was_empty;
> }
> EXPORT_SYMBOL(prepare_to_wait_exclusive);
> @@ -269,10 +256,8 @@ EXPORT_SYMBOL(init_wait_entry);
>
> long prepare_to_wait_event(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry, int state)
> {
> - unsigned long flags;
> - long ret = 0;
> + guard(spinlock_irqsave)(&wq_head->lock);
>
> - spin_lock_irqsave(&wq_head->lock, flags);
> if (signal_pending_state(state, current)) {
> /*
> * Exclusive waiter must not fail if it was selected by wakeup,
> @@ -287,19 +272,18 @@ long prepare_to_wait_event(struct wait_queue_head *wq_head, struct wait_queue_en
> * we fail.
> */
> list_del_init(&wq_entry->entry);
> - ret = -ERESTARTSYS;
> - } else {
> - if (list_empty(&wq_entry->entry)) {
> - if (wq_entry->flags & WQ_FLAG_EXCLUSIVE)
> - __add_wait_queue_entry_tail(wq_head, wq_entry);
> - else
> - __add_wait_queue(wq_head, wq_entry);
> - }
> - set_current_state(state);
> + return -ERESTARTSYS;
> }
> - spin_unlock_irqrestore(&wq_head->lock, flags);
>
> - return ret;
> + if (list_empty(&wq_entry->entry)) {
> + if (wq_entry->flags & WQ_FLAG_EXCLUSIVE)
> + __add_wait_queue_entry_tail(wq_head, wq_entry);
> + else
> + __add_wait_queue(wq_head, wq_entry);
> + }
> + set_current_state(state);
> +
> + return 0;
> }
> EXPORT_SYMBOL(prepare_to_wait_event);
>
> @@ -355,8 +339,6 @@ EXPORT_SYMBOL(do_wait_intr_irq);
> */
> void finish_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry)
> {
> - unsigned long flags;
> -
> __set_current_state(TASK_RUNNING);
> /*
> * We can check for list emptiness outside the lock
> @@ -372,9 +354,8 @@ void finish_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_en
> * the list).
> */
> if (!list_empty_careful(&wq_entry->entry)) {
> - spin_lock_irqsave(&wq_head->lock, flags);
> + guard(spinlock_irqsave)(&wq_head->lock);
> list_del_init(&wq_entry->entry);
> - spin_unlock_irqrestore(&wq_head->lock, flags);
> }
> }
> EXPORT_SYMBOL(finish_wait);
>
> base-commit: 328802738e1cd091d04076317f3c2174125c5916
Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>
prev parent reply other threads:[~2025-04-15 18:13 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-02 18:38 K Prateek Nayak
2025-04-15 18:13 ` Shrikanth Hegde [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f9f8144c-7666-4675-a17b-b31bed2db6c5@linux.ibm.com \
--to=sshegde@linux.ibm.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®