Not sure if you've seen it, but ipmi(4) has been disabled for over 12 years, because it's broken on some machines, so, this code is not necessarily guaranteed to be correct as-is.
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/i386/conf/GENERIC#rev1.632 http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/amd64/conf/GENERIC#rev1.238 C. On Wed, 2 Dec 2020 at 12:36, Scott Cheloha <[email protected]> wrote: > Hi, > > ipmi(4) is one of the few remaining callers of tsleep(9). I want to > convert it to use tsleep_nsec(9) but I need some clarification on what > the code in question is doing. > > In ipmi_poll_thread() we initialize all the sensors in a loop. > Between each get_sdr() call we tsleep(9) for 1 tick. > > So, I wonder: > > 1. Why do we sleep here? Is get_sdr() slow and we don't want to > hog the CPU? We block for a tick to let other stuff run? Or > is there some other reason? > > 2. Why are we spinning until tsleep(9) returns EWOULDBLOCK? Can > another thread interrupt the sleep with a wakeup(9)? > > marco@ added the tsleep(9) loop in this commit: > > > http://cvsweb.openbsd.org/src/sys/dev/ipmi.c?rev=1.59&content-type=text/x-cvsweb-markup > > Any ideas? > > My guess is that the attached diff will work fine but if we could > remove the loop while we're here that would simplify the code. > > Index: ipmi.c > =================================================================== > RCS file: /cvs/src/sys/dev/ipmi.c,v > retrieving revision 1.112 > diff -u -p -r1.112 ipmi.c > --- ipmi.c 29 Mar 2020 09:31:10 -0000 1.112 > +++ ipmi.c 2 Dec 2020 20:31:57 -0000 > @@ -1497,7 +1497,8 @@ ipmi_poll_thread(void *arg) > printf("%s: no SDRs IPMI disabled\n", DEVNAME(sc)); > goto done; > } > - while (tsleep(sc, PWAIT, "ipmirun", 1) != EWOULDBLOCK) > + while (tsleep_nsec(sc, PWAIT, "ipmirun", > + MSEC_TO_NSEC(1)) != EWOULDBLOCK) > continue; > } > > >
