Subject: Re: USB stack needs early review (Re: Someone should fix our USB stack...)
To: None <tech-kern@NetBSD.org>
From: der Mouse <mouse@Rodents.Montreal.QC.CA>
List: tech-kern
Date: 04/28/2007 11:07:06
>> while (! (sc->flags & SCF_INTERRUPTED))
>> ;
>> (Yes, it's somewhat unrealistic; top halves rarely busy-wait like
>> that. But that's an efficiency issue; absent volatile, there's a
>> correctness issue as well.)
> Yes, there's a correctness issue. If a top half is going to look at
> data that is shared with the bottom half, it must lock out the bottom
> half to do so.
This is not actually necessary. (If NetBSD wants to make it true by
fiat for NetBSD code, that's fine, but it should not be confused with
any underlying necessity.) Provided stores and reads are serialized in
some order, with each accessor's accesses performed in the order
they're issued, it's possible to interlock without additional help.
While this is not suitable for general-purpose use, it's perfectly
reasonable for cases where the types of contention that are likely to
arise are appropriate.
> That code is simply bogus; it needs to look more like this:
> s = splfoo()
> while (! (sc->flags & SCF_INTERRUPTED)) {
> tsleep();
> }
> splx(s);
Except it actually doesn't - provided sc->flags is volatile. And
indeed it quite likely is not a good idea for it to, if the loop does
not loop more than some small number of times on the average (or if
this is early enough that the scheduler behind tsleep() isn't up).
> ... noting that the bottom half which can write to sc->flags is
> blocked out while it is being examined.
Still doesn't help if the data being examined is file-static and does
not have its address stored anywhere global and thus is not modifiable
by tsleep() according to the C virtual machine; the compiler is then
perfectly within its rights to keep the flags word in a register across
the tsleep().
Unless it's volatile.
/~\ The ASCII der Mouse
\ / Ribbon Campaign
X Against HTML mouse@rodents.montreal.qc.ca
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B