Re: complete_all and "forever" completions

[Daniel Wagner]

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