> Date: Thu, 8 Oct 2015 18:12:26 +0200
> From: Mike Belopuhov <[email protected]>
>
> Is it possible that em_reset_hw is called a bit too early
> while we don't have full control of the driver?
I don't think that's a problem.
> Also it appears to me that while we're mi_switch'ing in the
> sched_barrier/sleep_finish another process can start running
> em_init since mi_switch will unlock all KERNEL_LOCKs that the
> process doing em_stop might hold. Now of course KASSERT would
> have fired if IFF_RUNNING would have been set, but perhaps we
> just haven't hit this yet. Shouldn't we do rwlock protection
> to make sure that while em_stop is not finished no other
> process can do em_stop and/or em_init?
I certainly couldn't completely convince myself that such a race
couldn't happen. That's why I put that KASSERT there. Adding an
rwlockmight indeed make sense. But I wonder if the locking should be
added in the layer above such that we don't have to replicate it in
all the drivers.
> diff --git sys/dev/pci/if_em.c sys/dev/pci/if_em.c
> index f2ddb32..7b24dce 100644
> --- sys/dev/pci/if_em.c
> +++ sys/dev/pci/if_em.c
> @@ -1559,13 +1559,14 @@ em_stop(void *arg, int softonly)
> timeout_del(&sc->timer_handle);
> timeout_del(&sc->tx_fifo_timer_handle);
>
> - if (!softonly) {
> + if (!softonly)
> em_disable_intr(sc);
> - em_reset_hw(&sc->hw);
> - }
>
> intr_barrier(sc->sc_intrhand);
>
> + if (!softonly)
> + em_reset_hw(&sc->hw);
> +
> KASSERT((ifp->if_flags & IFF_RUNNING) == 0);
>
> em_free_transmit_structures(sc);
>
