On 2020/12/02 14:04, Constantine A. Murenin wrote: > 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.
yes I have a recollection that it may have not worked on some machine (possibly an IBM server). That said I have had no problems on various Dell and all sorts of Supermicro boards where I've used it, and others have had similar results. (I would test the diff on some of them, but I am a bit serial-console- challenged at the moment on the machines with the right hardware to test..) > 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; > > } > > > > > >
