From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932140AbbJZM7w (ORCPT ); Mon, 26 Oct 2015 08:59:52 -0400 Received: from mail.bmw-carit.de ([62.245.222.98]:57370 "EHLO mail.bmw-carit.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932117AbbJZM7u (ORCPT ); Mon, 26 Oct 2015 08:59:50 -0400 X-CTCH-RefID: str=0001.0A0C0204.562E23C1.00F7,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0 Subject: Re: [PATCH v3 1/4] wait.[ch]: Introduce the simple waitqueue (swait) implementation To: Boqun Feng References: <1445326090-1698-1-git-send-email-daniel.wagner@bmw-carit.de> <1445326090-1698-2-git-send-email-daniel.wagner@bmw-carit.de> <20151026120426.GA1497@fixme-laptop.cn.ibm.com> CC: , , "Peter Zijlstra (Intel)" , Paul Gortmaker , Marcelo Tosatti , Paolo Bonzini , "Paul E. McKenney" , Thomas Gleixner From: Daniel Wagner X-Enigmail-Draft-Status: N1110 Message-ID: <562E23C0.9000500@bmw-carit.de> Date: Mon, 26 Oct 2015 13:59:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151026120426.GA1497@fixme-laptop.cn.ibm.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Boqun, On 10/26/2015 01:04 PM, Boqun Feng wrote: > On Tue, Oct 20, 2015 at 09:28:07AM +0200, Daniel Wagner wrote: >> + >> +/* >> + * The thing about the wake_up_state() return value; I think we can ignore it. >> + * >> + * 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) >> +{ >> + struct swait_queue *curr; >> + >> + list_for_each_entry(curr, &q->task_list, task_list) { >> + wake_up_process(curr->task); >> + list_del_init(&curr->task_list); >> + break; > > Just be curious, what's this break for? Or what's this loop(?) for? I have to guess here, since Peter wrote it. It looks like the function is based on __wake_up_common(). Though I agree the loop is not necessary and something like below should the trick. Unless I do not see something important. void swake_up_locked(struct swait_queue_head *q) { struct swait_queue *curr; if (list_emtpy(&q)) return; curr = list_first_entry(&q, typeof(*curr), task_list); wake_up_process(curr->task); list_del_init(&curr->task_list); } If Peter is not complaining I change swake_up_locked() for the next version. Thanks, Daniel