Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 19 Jan 2001 23:53:05 -0500
From:      "Bosko Milekic" <bmilekic@technokratis.com>
To:        <freebsd-arch@freebsd.org>
Cc:        <jasone@freebsd.org>
Subject:   wakeup_one() return value
Message-ID:  <000701c0829c$e0378d60$1f90c918@jehovah>
next in thread | raw e-mail | index | archive | help 
Hi,

    Here's a diff: http://people.freebsd.org/~bmilekic/code/wakeupone.diff
that makes wakeup_one() return an integer specifying whether a process was
actually taken off the sleep queue, or not. The reason for the proposed
change as to do with SMP and potential race conditions in certain situations.
For example, wakeup_one() is sometimes (and quite effectively) used in this
scenario:

thread 1:
---------

  mtx_enter(lock_A, foo);
  do stuff;
  slp_cnt++;
  if (msleep(foobar, lock_A, ..., some_time_period) == EWOULDBLOCK)
        slp_cnt--;
  do more stuff;
  mtx_exit(lock_A, foo);


thread 2:
---------

  mtx_enter(lock_A, foo);
  do stuff;
  if (slp_cnt >= 0) {
        wakeup_one(foobar);
        slp_cnt--;
  }
  mtx_exit(lock_A, foo);


The problem is that under some conditions, there is a race and slp_cnt can
get decremented twice. For example, consider slp_cnt == 0, and thread 1 comes
in and increments it to 1. It goes to sleep but wakes up and grabs sched_lock
(it hasn't yet regrabbed lock_A). At this time, thread 2 has lock_A and is
spinning on sched_lock in wakeup_one(). But, thread 1 has timed out, and
removes itself from the sleep queue; it returns EWOULDBLOCK, so slp_cnt goes
down to 0. sched_lock is dropped, the thread blocks waiting for thread 2 to
let go of lock_A so it can "do more stuff." Meanwhile, thread 2 gets
sched_lock and does a wakeup_one() (effectively a no-op) and decrements
slp_cnt again. There's an underflow.

The patch will allow this, for thread 2:

mtx_enter(lock_A, foo);
do stuff;
if (slp_cnt >= 0) {
    if (wakeup_one(foobar))
        slp_cnt--;
}
mtx_exit(lock_A, foo);

Please note that there should be no issue with wakeup(), because wakeup()
will take away all processes matching `foobar' from the sleepqueue, so nobody
implements a slp_cnt for uses of wakeup().

Any objections? If this goes in, we should probably do similar to the
cond-var interface function that wakes up one sleeper.

Re: diff; I cast (void)wakeup_one() to all the wakeup_one()s for now, which
means we should go ahead and review them when/if this goes in. I'll take care
of the sys/mbuf.h case, but I think that at least softupdates should be
checked too (I saw some counters in there). If someone doesn't like the
casts, let me know.

Thanks,
Bosko.




To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?000701c0829c$e0378d60$1f90c918>