On 09/11/2017 10:18, Peter Xu wrote:
> Let swake_up() to return whether any of the waiters is waked up. One use
> case of it would be:
>
> if (swait_active(wq)) {
> swake_up(wq);
> // do something when waiter is waked up
> waked_up++;
> }
>
> Logically it's possible that when reaching swake_up() the wait queue is
> not active any more, and here doing something like waked_up++ would be
> inaccurate. To correct it, we need an atomic version of it.
>
> With this patch, we can simply re-write it into:
>
> if (swake_up(wq)) {
> // do something when waiter is waked up
> waked_up++;
> }
>
> After all we are checking swait_active() inside swake_up() too.
Better subject:
sched/swait: make swake_up() return whether there were any waiters
I like this patch.
>
> CC: Ingo Molnar <mingo@xxxxxxxxxx>
> CC: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> CC: Peter Xu <peterx@xxxxxxxxxx>
> CC: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
> include/linux/swait.h | 4 ++--
> kernel/sched/swait.c | 21 ++++++++++++++++-----
> 2 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/swait.h b/include/linux/swait.h
> index c1f9c62a8a50..c55171280ac8 100644
> --- a/include/linux/swait.h
> +++ b/include/linux/swait.h
> @@ -84,9 +84,9 @@ static inline int swait_active(struct swait_queue_head *q)
> return !list_empty(&q->task_list);
> }
>
> -extern void swake_up(struct swait_queue_head *q);
> +extern bool swake_up(struct swait_queue_head *q);
> extern void swake_up_all(struct swait_queue_head *q);
> -extern void swake_up_locked(struct swait_queue_head *q);
> +extern bool swake_up_locked(struct swait_queue_head *q);
>
> extern void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait);
> extern void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state);
> diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
> index 3d5610dcce11..31f8d677c690 100644
> --- a/kernel/sched/swait.c
> +++ b/kernel/sched/swait.c
> @@ -16,29 +16,40 @@ EXPORT_SYMBOL(__init_swait_queue_head);
> * If for some reason it would return 0, that means the previously waiting
> * task is already running, so it will observe condition true (or has already).
> */
> -void swake_up_locked(struct swait_queue_head *q)
> +bool swake_up_locked(struct swait_queue_head *q)
> {
> struct swait_queue *curr;
>
> if (list_empty(&q->task_list))
> - return;
> + return false;
>
> curr = list_first_entry(&q->task_list, typeof(*curr), task_list);
> wake_up_process(curr->task);
> list_del_init(&curr->task_list);
> + return true;
> }
> EXPORT_SYMBOL(swake_up_locked);
>
> -void swake_up(struct swait_queue_head *q)
> +/**
> + * swake_up - Wake up one process on the waiting list
> + * @q: the waitqueue to wake up
> + *
> + * Returns true if some process is waked up, otherwise false if there
> + * is no waiter to wake up.
> + */
> +bool swake_up(struct swait_queue_head *q)
> {
> + bool ret;
> unsigned long flags;
>
> if (!swait_active(q))
> - return;
> + return false;
>
> raw_spin_lock_irqsave(&q->lock, flags);
> - swake_up_locked(q);
> + ret = swake_up_locked(q);
> raw_spin_unlock_irqrestore(&q->lock, flags);
> +
> + return ret;
> }
> EXPORT_SYMBOL(swake_up);
>
>
