On Wednesday 05 December 2001 06:12, YAMAMOTO Takashi wrote:

>
> but i wonder if it is enough.
> the code I know which can produce bad code is
> free_lock in ufs/ffs/ffs_softdep.c .
> the problem is re-ordering "lk->lkt_held = -1"
> and "cpl = ncpl" in splx.
> since cpl is already volatile,
> i'm not sure why marking ncpl as volatile suppress re-ordering
> in this case...

You are referring to "FREE_LOCK(&lk)" calls, right? (as opposed to the rest 
of FREE_LOCK calls)

        movl lk,%edx
        movl %edx,cpl
        movl ipending,%eax
        movl $-1,lk+4			// HERE lk.lkt_held is updated after cpl
        xorl $-1,%edx
        testl %eax,%edx
        je .L443
        call Xspllower

If so, shouldn't "static struct lockit lk" be declared volatile?

C is carefull when dealing with code that uses pointers because they may 
often point to the same location. And I think this is the case with other 
FREE_LOCK calls - "cpl = ncpl" and "lk->lkt_held = 1" is not swapped when lk 
is unknown pointer. When lk is & of known struct, compiler can reorder the 
code.

So it is actually blind luck that good code is often generated - and rest of 
the kernel is probably also affected. I think adding asm volatile("") will 
save us from lot's of other nasty bugs in the future.

On the other side, one could argue that *all* variables that could be 
reffered by asynchronously called code should be declared volatile.

Unfortunatelly both solutions are clearly suboptimal.


Greetings,


	Witold J. Wnuk



PS: I have a theoretical question - *if* it would be possible to overcome 
some C issues and enhance its functionality by using cfront-style 
preprocesor, would such solution be considered for NetBSD?