Re: CVS commit: src/sys/dev/pci

2017-02-01 Thread Taylor R Campbell
> Date: Thu, 2 Feb 2017 03:57:21 +
> From: co...@sdf.org
> 
> On Thu, Feb 02, 2017 at 03:41:22AM +, Jonathan A. Kollasch wrote:
> > @@ -432,6 +433,10 @@ wpi_detach(device_t self, int flags __un
> > pci_intr_disestablish(sc->sc_pct, sc->sc_ih);
> > sc->sc_ih = NULL;
> > }
> > +   if (sc->sc_pihp != NULL) {
> > +   pci_intr_release(sc->sc_pct, sc->sc_pihp, 1);
> > +   sc->sc_pihp = NULL;
> > +   }
> > mutex_enter(>sc_rsw_mtx);
> > sc->sc_dying = 1;
> > cv_signal(>sc_rsw_cv);
> > 
> 
> hmm, not introduced by you now, but is it safe that this function is 
> mutex_entering so late?
> shouldn't it mutex enter and set sc_dying first thing?

In this case, the purpose is of setting sc->sc_dying to cause
wpi_rsw_thread to terminate.  The only interesting thing that thread
does is wpi_getrfkill.  All that wpi_getrfkill uses is the device
register mapping (via wpi_mem_lock and wpi_mem_read) and sc->sc_rsw,
which wpi_detach tears down only after wpi_rsw_thread has terminated.
So I don't think there's a problem with that ordering.


Re: CVS commit: src/sys/dev/pci

2017-02-01 Thread coypu
On Thu, Feb 02, 2017 at 03:41:22AM +, Jonathan A. Kollasch wrote:
> @@ -432,6 +433,10 @@ wpi_detach(device_t self, int flags __un
>   pci_intr_disestablish(sc->sc_pct, sc->sc_ih);
>   sc->sc_ih = NULL;
>   }
> + if (sc->sc_pihp != NULL) {
> + pci_intr_release(sc->sc_pct, sc->sc_pihp, 1);
> + sc->sc_pihp = NULL;
> + }
>   mutex_enter(>sc_rsw_mtx);
>   sc->sc_dying = 1;
>   cv_signal(>sc_rsw_cv);
> 

hmm, not introduced by you now, but is it safe that this function is 
mutex_entering so late?
shouldn't it mutex enter and set sc_dying first thing?

thanks