Steve Woodford <scw@NetBSD.org> writes:

> On Wednesday 07 April 2004 12:35 pm, Martin Husemann wrote:
> > On Wed, Apr 07, 2004 at 09:41:01AM +0300, Jukka Andberg wrote:
> > > 2) chp->ch_flags is read into a register
> > > 3) splx()
> > > 4) wdcintr() runs, changing the ch_flags
> >
> > Should splx() imply a memory clobber?
> 
> No. That it does on some architectures is purely an implementation 
> detail.
> 
> The right fix is to declare ch_flags to be volatile. Any variable shared 
> between the top/bottom halves of a driver should be declared volatile 
> unless there is a very good reason not to.

That feels wrong to me; perhaps it's too much work in the world of
pthread concrurrency, where use of "volatile" is considered to be the
wrong answer to sharing issues.

We need to communicate to the compiler that chp->ch_flags can be
changed by other threads of execution when spl's aren't held; as long
as the top-half code sticks to only accessing the data while an spl is
held, this is equivalent to making the compiler believe that splNNN()
can alter globally-visible data - analagous to the way that the
compiler has to think that pthread_mutex_lock() can alter
globally-visible data (which works because pthread_mutex_lock() is a
genuine function call, and gcc doesn't do interprocedural
pointer-access analysis).

Unfortunately, the clobber expressions avaliable for inline asm only
seem to be able to express "clobber all memory", not "clobber all
globally-visible memory", so the tradeoff is between killing all
caching in registers across spl calls (including locals which might
not have to go into memory at all otherwise), and forcing memory
operations for every load of a field shared by the top and bottom
layers (or the spl() operations could be made into non-inline
functions). I'm not at all sure which option is better.

        - Nathan