On 07/09/21(Tue) 18:03, Stefan Sperling wrote: > On Tue, Sep 07, 2021 at 05:16:52PM +0200, Martin Pieuchot wrote: > > On 07/09/21(Tue) 15:48, Stefan Sperling wrote: > > > This patch makes iwx(4) resume reliably for me. > > > > > > There were missing splnet() calls which leads to an obvious race > > > between the interrupt handler and the code which triggers firmware > > > loading and then sleeps to wait for confirmation. > > > This patch adds the missing splnet(). > > > > > > However, even with splnet() protection added I need to add the > > > following two lines of code to make the patch work reliably: > > > > > > /* wait for the firmware to load */ > > > for (w = 0; !sc->sc_uc.uc_intr && w < 10; w++) { > > > err = tsleep_nsec(&sc->sc_uc, 0, "iwxuc", MSEC_TO_NSEC(100)); > > > + /* XXX This should not be needed, should it: */ > > > + if (err == EWOULDBLOCK && sc->sc_uc.uc_intr) > > > + err = 0; > > > } > > > if (err || !sc->sc_uc.uc_ok) > > > printf("%s: could not load firmware, %d\n", DEVNAME(sc), err); > > > > > > Which seems odd. I would expect tsleep to return EWOULDBLOCK only when > > > the interrupt handler did not already set uc_intr and call wakeup(). > > > > That suggests the timeout fires before the wakeup(9). > > Yes, it does. > > But how could uc_intr already be set to 1 in that case?
Is it set before the timeout fires or before tsleep(9) returns?