From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751519AbbJLJRV (ORCPT ); Mon, 12 Oct 2015 05:17:21 -0400 Received: from mail.bmw-carit.de ([62.245.222.98]:40427 "EHLO mail.bmw-carit.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751401AbbJLJRT (ORCPT ); Mon, 12 Oct 2015 05:17:19 -0400 X-CTCH-RefID: str=0001.0A0C0204.561B7A9B.020E,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0 Subject: Re: [PATCH v1 3/8] sched/completion: convert completions to use simple wait queues To: Peter Zijlstra References: <1441800334-16609-1-git-send-email-daniel.wagner@bmw-carit.de> <1441800334-16609-4-git-send-email-daniel.wagner@bmw-carit.de> <20150909142608.GP12596@twins.programming.kicks-ass.net> CC: From: Daniel Wagner X-Enigmail-Draft-Status: N1110 Message-ID: <561B7A9A.3020904@bmw-carit.de> Date: Mon, 12 Oct 2015 11:17:14 +0200 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: <20150909142608.GP12596@twins.programming.kicks-ass.net> 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 On 09/09/2015 04:26 PM, Peter Zijlstra wrote: > On Wed, Sep 09, 2015 at 02:05:29PM +0200, Daniel Wagner wrote: >> @@ -50,10 +50,10 @@ void complete_all(struct completion *x) >> { >> unsigned long flags; >> >> - spin_lock_irqsave(&x->wait.lock, flags); >> + raw_spin_lock_irqsave(&x->wait.lock, flags); >> x->done += UINT_MAX/2; >> - __wake_up_locked(&x->wait, TASK_NORMAL, 0); >> - spin_unlock_irqrestore(&x->wait.lock, flags); >> + swake_up_locked(&x->wait); >> + raw_spin_unlock_irqrestore(&x->wait.lock, flags); >> } >> EXPORT_SYMBOL(complete_all); > > I don't think that's correct; __wake_up_locked(.nr=0) would wake all > waiters, where swake_up_locked() will only wake one. I read that x->done should be protected via wait.lock during the whole operation. swake_up_all() will release and reacquire the lock while processing the all waiters. So we need to get Could we play a trick like setting the highest bit in done for indicating the complete_all() operation. The UINT_MAX/2 update looks like do this by setting a value which has the biggest offset from 0 (but why adding instead of just going for assigning...). cheers, daniel