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?
