> From: "Constantine A. Murenin" <[email protected]>
> Date: Wed, 2 Dec 2020 14:04:52 -0800
> 
> 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
>

The driver is actually enabled on arm64.  And I'll probably enable it
on powerpc64 at some point.

> 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;
> >         }
> >
> >
> >
> 

Reply via email to