Re: Synchronizing if_xennet_xenbus detach with event-handling

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Fri, Feb 05, 2016 at 01:09:28PM -0800, Rohan Desai wrote:
> I'm getting a panic due to a race between the detach path
> (xennet_xenbus_detach) and event handling (xennet_handler called from
> triggered event channel). xennet_xenbus_detach frees up the tx ring and
> xennet_handler tries to access it. I'm not sure what event the event
> channel is being poked for (tx complete, rx ready, other). Does anybody
> know how these two code-paths are supposed to be synchronized ?

xennet_xenbus_detach() runs at splnet(), so this should be enough to prevent
the event handler from being called.

But I see where the race could be: we sleep waiting for the rx grant
ref to be released, the event handler could be called at this time.
I guess just removing the interrupt handler earlier, like in the
attached patch, is enough to fix it. Can you test ?

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: if_xennet_xenbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/if_xennet_xenbus.c,v
retrieving revision 1.65
diff -u -p -u -r1.65 if_xennet_xenbus.c
--- if_xennet_xenbus.c	19 Nov 2015 17:01:40 -0000	1.65
+++ if_xennet_xenbus.c	6 Feb 2016 16:51:46 -0000
@@ -443,6 +443,9 @@ xennet_xenbus_detach(device_t self, int 
 
 	/* Unhook the entropy source. */
 	rnd_detach_source(&sc->sc_rnd_source);
+	/* unhook interrupt handlers */
+	softint_disestablish(sc->sc_softintr);
+	event_remove_handler(sc->sc_evtchn, &xennet_handler, sc);
 
 	while (xengnt_status(sc->sc_tx_ring_gntref)) {
 		tsleep(xennet_xenbus_detach, PRIBIO, "xnet_txref", hz/2);
@@ -456,8 +459,6 @@ xennet_xenbus_detach(device_t self, int 
 	xengnt_revoke_access(sc->sc_rx_ring_gntref);
 	uvm_km_free(kernel_map, (vaddr_t)sc->sc_rx_ring.sring, PAGE_SIZE,
 	    UVM_KMF_WIRED);
-	softint_disestablish(sc->sc_softintr);
-	event_remove_handler(sc->sc_evtchn, &xennet_handler, sc);
 	splx(s0);
 
 	pmf_device_deregister(self);