[PATCH 10/25] mciseq: Limit concurrency when starting to play.

[David Laight]

On Tue, Oct 09, 2012 at 04:05:09PM +0200, Alexandre Julliard wrote:
...
> >> Also there's no such thing as
> >>"conceptually volatile", unless you add explicit memory barriers.
> > Forget about "conceptually volatile".  I meant to express the idea
> > that some variable may be updated from another thread.  This has
> > nothing to do with the C keyword of that name.
> >
> > Once again I read some articles about memory barriers and fail to see
> > what's wrong with the final code (after patch 21/25):
> >
> > 	if (wmm->dwStatus == MCI_MODE_STOP || wmm->dwStatus == MCI_MODE_NOT_READY)
> > 	    break;
> > 	else if (wmm->dwStatus != MCI_MODE_PLAY || recheck)
> > 	    continue;
> 
> That depends on what state transitions are possible and what the
> behavior should be in various states. I haven't analyzed that in detail.

If the dwStatus field ISN'T volatile then the compile might generate
code for the above that reads it once, twice, or three times.

If we assume that reads of it are atomic (ie it is not larger than a
machine word, and is aligned) then there is no need to acquire any mutex
across the read - unless you are also checking another variable.

The above code almost certainly requires that only a single read be done.
This is likely to be the case if the code is optimised, and unlikely if not.
So in the above the repeated reads probably technically require a lock.

Better to code as:
	status = wmm->dwStatus;
	if (...)
but even then I think the compiler is allowed to perform extra reads.
It might, for example, do so if the condition was complicayted and 
there was a lot of pressure on registers - so instead of spilling 'status'
to stack, it would re-read it.

With gcc you can force it to only to one read by adding:
	asm volatile("":::"memory");
before the first if.
The "memory" constraint of the asm statement tells gcc that it might
change any memory location - thus acting as a barrier.

The only portable way is with volatile.

	David

-- 
David Laight: david at l8s.co.uk