Re: the new read/write semaphores in 2.3.30

Benjamin C.R. LaHaise (blah@kvack.org)
Wed, 8 Dec 1999 18:55:27 -0500 (EST)


On Wed, 8 Dec 1999, Dimitris Michailidis wrote:

>
> On 08-Dec-99 Benjamin C.R. LaHaise wrote:
> > On Wed, 8 Dec 1999, Dimitris Michailidis wrote:
> >
> >> An 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);
>
> Suppose count = 0 with a writer finishing down_write_failed_biased and two
> readers on sem->wait. If you just do wake_up(&sem->wait) you'll wake up
> both and at most one of them can bias the lock. So it appears to me that we
> need to wake up just one process at that point. wake_up, the way it is now,
> doesn't allow this since both readers are non-exclusive, but this is the
> desired behavior, no?

Sorry, I misquoted -- the no was to the change in the code's if condition
from >= 0 to <= 0. As for wake_up versus a wake one, it is desirable to
wake up only one process. As to whether that process is a reader or a
writer is another question that in some cases should be decided by the
scheduler.

> > 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.
>
> But this is an artifact of the use of wake_up(). If the desired behavior is
> to wake up only one process, be it reader or writer, we should use something
> that provides this behavior. It doesn't have to be wake_up.

It also prevents a race condition whereby the lock is left unlocked with
all contenders sleeping. See below.

> My contention is that you want to wake up just one process in
> down_write_failed_biased, but all the readers + one writer in
> rwsem_wake_readers. Is this incorrect?

Half. rwsem_wake_readers has to wake all readers plus a writer -- this is
right. But, the way the down_read_failed case is currently precludes
waking only one process right now because it means that not all readers
will be woken if the lock is completely unlocked before a reader comes
along to bias the lock. This is because the code only does a wake_up on
the state transitions involving a change to or from the biased state (the
locks only have 3 states: unlocked, locked and biased).

-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/