Date: Thu, 27 Oct 2016 11:51:28 +0200
From: Daniel Wagner <wagi@monom.org>
To: Nicholas Mc Guire <der.herr@hofr.at>
Cc: Peter Zijlstra <peterz@infradead.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>, Tejun Heo <tj@kernel.org>,
        computersforpeace@gmail.com, Ingo Molnar <mingo@kernel.org>
Subject: Re: complete_all and "forever" completions
Message-ID: <20161027095127.gl464wgwonssk7ry@nobby.bmw-carit.de>
References: <20161025223054.GA22917@dtor-ws>
 <20161026084535.GX3102@twins.programming.kicks-ass.net>
 <20161026091519.GA577@osadl.at>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20161026091519.GA577@osadl.at>
User-Agent: NeoMutt/20161003 (1.7.0)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1970
Lines: 54

On Wed, Oct 26, 2016 at 09:15:19AM +0000, Nicholas Mc Guire wrote:
> On Wed, Oct 26, 2016 at 10:45:35AM +0200, Peter Zijlstra wrote:
> > On Tue, Oct 25, 2016 at 03:30:54PM -0700, Dmitry Torokhov wrote:
> > > Or do we need something like this in
> > > do_wait_for_common():
> > > 
> > > 	if (x->done < UINT_MAX/2)
> > > 		x->done--;
> > 
> > Depends a bit, do you really want this? Seems a bit daft to keep asking
> > if its done already, seems like a waste of cycles to me.
> > 
> 
> I would claim that if you have a complete_all() (done=2147483648) and you
> actually did manage to decrement it to 0 over time so a call finally blocks
> (presumably for ever) this would be uncovering a deisgn bug in the use of
> completion as such a setup does not make any sense (Or Im just not creative
> enough to think of such a situation).

I am reviewing all the complete_all() users in order to figure out if
we could weaken the garantees which complete_all() gives you: can be
used in hard irq context and irq disabled context. But that is a
different story.

So while doing the review I found things like

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:

vchiq_arm_init_state() {
	[...]

		init_completion(&arm_state->vc_resume_complete);
		/* Initialise to 'done' state.  We only want to block on resume
		 * completion while videocore is suspended. */
		set_resume_state(arm_state, VC_RESUME_RESUMED);

		init_completion(&arm_state->resume_blocker);
		/* Initialise to 'done' state.  We only want to block on this
		 * completion while resume is blocked */
		complete_all(&arm_state->resume_blocker);

		init_completion(&arm_state->blocked_blocker);
		/* Initialise to 'done' state.  We only want to block on this
		 * completion while things are waiting on the resume blocker */
		complete_all(&arm_state->blocked_blocker);

	[...]
}

If I read this corredtly, there are some 'interesting' uses of
completion where you might run into limits.

cheers,
daniel