Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: acpiec: try a short busy-wait first
To:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Cc:
jcs@jcs.org, tech@openbsd.org
Date:
Tue, 11 Nov 2025 17:25:08 +0100

Download raw body.

Thread
> Date: Tue, 11 Nov 2025 17:10:05 +0100
> From: Claudio Jeker <cjeker@diehard.n-r-g.com>
> 
> On Tue, Nov 11, 2025 at 05:04:37PM +0100, Mark Kettenis wrote:
> > > Date: Mon, 10 Nov 2025 21:12:47 -0600
> > > From: joshua stein <jcs@jcs.org>
> > > 
> > > A single execution of acpisbs_read() does 1027 EC reads, 299 writes, 
> > > and takes about 5980 milliseconds.  It has to do this on the ACPI 
> > > task queue thread which means anything else behind it has to wait up 
> > > to nearly 6 seconds to run.
> > > 
> > > If we busy-wait for a wee bit before falling back on tsleep, the 
> > > same acpisbs function with the same number of EC reads/writes 
> > > finishes in 125 milliseconds.
> > 
> > I guess doing this should be safe.  Before I give an ok; did you
> > try using tsleep_nsec() with a shorter timeout?
> 
> I doubt that changes anything, tsleep_nsec still rounds up to 1 tick minimum.

Ah, yes, for some reason I thought we had moved to more granular
timeouts already.

ok kettenis@

> > > diff --git sys/dev/acpi/acpiec.c sys/dev/acpi/acpiec.c
> > > index e6add9e7ef0..fc490ecbd33 100644
> > > --- sys/dev/acpi/acpiec.c
> > > +++ sys/dev/acpi/acpiec.c
> > > @@ -91,7 +91,8 @@ void
> > >  acpiec_wait(struct acpiec_softc *sc, uint8_t mask, uint8_t val)
> > >  {
> > >  	static int acpiecnowait;
> > > -	uint8_t		stat;
> > > +	int tries = 0;
> > > +	uint8_t	stat;
> > >  
> > >  	dnprintf(40, "%s: EC wait_ns for: %b == %02x\n",
> > >  	    DEVNAME(sc), (int)mask,
> > > @@ -100,7 +101,7 @@ acpiec_wait(struct acpiec_softc *sc, uint8_t mask, uint8_t val)
> > >  	while (((stat = acpiec_status(sc)) & mask) != val) {
> > >  		if (stat & EC_STAT_SCI_EVT)
> > >  			sc->sc_gotsci = 1;
> > > -		if (cold || (stat & EC_STAT_BURST))
> > > +		if (cold || (stat & EC_STAT_BURST) || tries++ < 300)
> > >  			delay(1);
> > >  		else
> > >  			tsleep(&acpiecnowait, PWAIT, "acpiec", 1);
> > > 
> > > 
> > 
> 
> -- 
> :wq Claudio
>