From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754072AbdKIKHE (ORCPT ); Thu, 9 Nov 2017 05:07:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44882 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752920AbdKIKHC (ORCPT ); Thu, 9 Nov 2017 05:07:02 -0500 Subject: Re: [PATCH 1/2] sched/swait: allow swake_up() to return To: Peter Xu , kvm@vger.kernel.org Cc: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Ingo Molnar , Peter Zijlstra , linux-kernel@vger.kernel.org References: <20171109091854.24367-1-peterx@redhat.com> <20171109091854.24367-2-peterx@redhat.com> From: Paolo Bonzini Message-ID: Date: Thu, 9 Nov 2017 11:06:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171109091854.24367-2-peterx@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 09 Nov 2017 10:07:02 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > CC: Peter Zijlstra > CC: Peter Xu > CC: linux-kernel@vger.kernel.org > Signed-off-by: Peter Xu > --- > 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); > >