> On 08-Dec-99 Benjamin C.R. LaHaise wrote:
> > On Wed, 8 Dec 1999, Dimitris Michailidis wrote:
> >
> >> If I understood the code right sem->count cannot be >0 at that point
> >> because
> >> the writer that is executing that code has been granted the semaphore, so
> >> this should trigger BUG().
>
> So
>
> if (atomic_read(&sem->count) >= 0)
> wake_up(&sem->wait);
>
> should be
>
> if (atomic_read(&sem->count) <= 0)
> wake_up(&sem->wait);
> else
> BUG();
> Right? I don't even think you need a wake_up if sem->count < 0. And that
> wake_up should wake up just one process so there's no stampede?
No, if changed at all, it should be:
if (atomic_read(&sem->count) == 0)
wake_up(&sem->wait);
And the paranoid BUG() case would just be an additional
if (atomic_read(&sem->count) > 0)
BUG();
Ie, the current code is correct assuming that count has not been corrupted
to be > 0 at this point, which it can't be since write_bias_granted will
only be called if an atomic operation on sem->count resulted in the zero
flag being set. Having count < 0 means that someone else has already
biased the lock and therefore we don't want to perform the wakeup.
> > Of course, this will also wake one writer
> > who is waiting, leading to a small stampede type race if the writer gets
> > in before the readers.
>
> This can happen but you need to wake up a writer at that point,
> otherwise when the readers exit they won't know that a writer is waiting the
> way the code is now.
At this point the lock is unbiased, and this will wake up one or more
readers (since they're not exclusive) and one writer (assuming lots of
contention, which is the only case for which stampedes are interesting).
Ideally, only 1 process would be woken up here to bias the lock, but we
can't since readers *must* be added non-exclusively.
-ben
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/