Subject: ath shutdown lockup
To: None <tech-net@netbsd.org>
From: Sean Boudreau <seanb@qnx.com>
List: tech-net
Date: 07/11/2006 10:25:50
Hi:

I'm seeing a lockup occuring on 'slow' machines
when they are shutdown.  This stems from the
fact that redundant shutdown hooks are being
registered for an ath interface: one in ath.c
itself and one in the parent bus layer (if_ath_pci.c
in this case).  This is tickling the bus lock up
that the comment ath_stop() describes.

The following diff removes the shutdown hook in
ath.c and consistently establishes it in the
bus layer.

Any comments before I commit?

Regards,

-seanb


Index: arch/mips/atheros/dev/if_ath_arbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/mips/atheros/dev/if_ath_arbus.c,v
retrieving revision 1.4
diff -c -r1.4 if_ath_arbus.c
*** arch/mips/atheros/dev/if_ath_arbus.c	5 Jun 2006 05:14:38 -0000	1.4
--- arch/mips/atheros/dev/if_ath_arbus.c	11 Jul 2006 14:03:55 -0000
***************
*** 73,78 ****
--- 73,79 ----
  	bus_space_handle_t	sc_ioh;
  	void			*sc_ih;
  	struct ar531x_config	sc_config;
+ 	void			*sc_sdhook;
  };
  
  static int	ath_arbus_match(struct device *, struct cfdata *, void *);
***************
*** 102,108 ****
  	struct ath_softc *sc;
  	struct arbus_attach_args *aa;
  	const char *name;
- 	void *hook;
  	int rv;
  	uint16_t devid;
  	uint32_t rev;
--- 103,108 ----
***************
*** 156,163 ****
  		goto err;
  	}
  
! 	hook = shutdownhook_establish(ath_arbus_shutdown, asc);
! 	if (hook == NULL) {
  		aprint_error("%s: couldn't establish shutdown hook\n",
  		    sc->sc_dev.dv_xname);
  		goto err;
--- 156,163 ----
  		goto err;
  	}
  
! 	asc->sc_sdhook = shutdownhook_establish(ath_arbus_shutdown, asc);
! 	if (asc->sc_sdhook == NULL) {
  		aprint_error("%s: couldn't establish shutdown hook\n",
  		    sc->sc_dev.dv_xname);
  		goto err;
***************
*** 174,179 ****
--- 174,180 ----
  {
  	struct ath_arbus_softc *asc = (struct ath_arbus_softc *)self;
  
+ 	shutdownhook_disestablish(asc->sc_sdhook);
  	ath_detach(&asc->sc_ath);
  	arbus_intr_disestablish(asc->sc_ih);
  
Index: dev/cardbus/if_ath_cardbus.c
===================================================================
RCS file: /cvsroot/src/sys/dev/cardbus/if_ath_cardbus.c,v
retrieving revision 1.14
diff -c -r1.14 if_ath_cardbus.c
*** dev/cardbus/if_ath_cardbus.c	7 Jun 2006 15:30:48 -0000	1.14
--- dev/cardbus/if_ath_cardbus.c	11 Jul 2006 14:04:08 -0000
***************
*** 112,122 ****
--- 112,124 ----
  	int	sc_intrline;		/* interrupt line */
  	bus_space_tag_t sc_iot;
  	bus_space_handle_t sc_ioh;
+ 	void	*sc_sdhook;
  };
  
  int	ath_cardbus_match(struct device *, struct cfdata *, void *);
  void	ath_cardbus_attach(struct device *, struct device *, void *);
  int	ath_cardbus_detach(struct device *, int);
+ void	ath_cardbus_shutdown(void *arg);
  
  CFATTACH_DECL(ath_cardbus, sizeof(struct ath_cardbus_softc),
      ath_cardbus_match, ath_cardbus_attach, ath_cardbus_detach, ath_activate);
***************
*** 166,171 ****
--- 168,179 ----
  	sc->sc_disable = ath_cardbus_disable;
  	sc->sc_power = ath_cardbus_power;
  
+ 	csc->sc_sdhook = shutdownhook_establish(ath_cardbus_shutdown, csc);
+ 	if (csc->sc_sdhook == NULL) {
+ 		aprint_error("couldn't establish shutdown hook\n");
+ 		return;
+ 	}
+ 
  	/*
  	 * Map the device.
  	 */
***************
*** 181,186 ****
--- 189,195 ----
  	else {
  		printf("%s: unable to map device registers\n",
  		    sc->sc_dev.dv_xname);
+ 		shutdownhook_disestablish(csc->sc_sdhook);
  		return;
  	}
  
***************
*** 221,226 ****
--- 230,237 ----
  		panic("%s: data structure lacks", sc->sc_dev.dv_xname);
  #endif
  
+ 	shutdownhook_disestablish(csc->sc_sdhook);
+ 
  	rv = ath_detach(sc);
  	if (rv)
  		return (rv);
***************
*** 241,246 ****
--- 252,267 ----
  	return (0);
  }
  
+ void
+ ath_cardbus_shutdown(void *arg)
+ {
+ 	struct ath_cardbus_softc *csc;
+ 
+ 	csc = arg;
+ 
+ 	ath_shutdown(&csc->sc_ath);
+ }
+ 
  int
  ath_cardbus_enable(struct ath_softc *sc)
  {
Index: dev/ic/ath.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/ath.c,v
retrieving revision 1.75
diff -c -r1.75 ath.c
*** dev/ic/ath.c	8 Jun 2006 22:42:24 -0000	1.75
--- dev/ic/ath.c	11 Jul 2006 14:04:12 -0000
***************
*** 640,652 ****
  
  #ifdef __NetBSD__
  	sc->sc_flags |= ATH_ATTACHED;
- 	/*
- 	 * Make sure the interface is shutdown during reboot.
- 	 */
- 	sc->sc_sdhook = shutdownhook_establish(ath_shutdown, sc);
- 	if (sc->sc_sdhook == NULL)
- 		printf("%s: WARNING: unable to establish shutdown hook\n",
- 			sc->sc_dev.dv_xname);
  	sc->sc_powerhook = powerhook_establish(ath_power, sc);
  	if (sc->sc_powerhook == NULL)
  		printf("%s: WARNING: unable to establish power hook\n",
--- 640,645 ----
***************
*** 714,720 ****
  	if_detach(ifp);
  	splx(s);
  	powerhook_disestablish(sc->sc_powerhook);
- 	shutdownhook_disestablish(sc->sc_sdhook);
  
  	return 0;
  }
--- 707,712 ----
Index: dev/ic/athvar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/athvar.h,v
retrieving revision 1.18
diff -c -r1.18 athvar.h
*** dev/ic/athvar.h	5 Jun 2006 05:15:31 -0000	1.18
--- dev/ic/athvar.h	11 Jul 2006 14:04:12 -0000
***************
*** 288,294 ****
  	HAL_NODE_STATS		sc_halstats;	/* station-mode rssi stats */
  	struct callout		sc_scan_ch;	/* callout handle for scan */
  	struct callout		sc_dfs_ch;	/* callout handle for dfs */
- 	void			*sc_sdhook;	/* shutdown hook */
  	void			*sc_powerhook;	/* power management hook */
  	u_int			sc_flags;	/* misc flags */
  };
--- 288,293 ----
Index: dev/pci/if_ath_pci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_ath_pci.c,v
retrieving revision 1.14
diff -c -r1.14 if_ath_pci.c
*** dev/pci/if_ath_pci.c	20 Jun 2006 14:38:34 -0000	1.14
--- dev/pci/if_ath_pci.c	11 Jul 2006 14:04:14 -0000
***************
*** 96,101 ****
--- 96,102 ----
  	void			*sc_ih;		/* interrupt handler */
  	bus_space_tag_t		sc_iot;
  	bus_space_handle_t	sc_ioh;
+ 	void			*sc_sdhook;
  };
  
  #define	BS_BAR	0x10
***************
*** 168,174 ****
  	struct pci_attach_args *pa = aux;
  	pci_chipset_tag_t pc = pa->pa_pc;
  	pci_intr_handle_t ih;
- 	void *shook;
  	void *phook;
  	const char *intrstr = NULL;
  
--- 169,174 ----
***************
*** 213,220 ****
  
  	sc->sc_dmat = pa->pa_dmat;
  
! 	shook = shutdownhook_establish(ath_pci_shutdown, psc);
! 	if (shook == NULL) {
  		aprint_error("couldn't make shutdown hook\n");
  		goto bad3;
  	}
--- 213,220 ----
  
  	sc->sc_dmat = pa->pa_dmat;
  
! 	psc->sc_sdhook = shutdownhook_establish(ath_pci_shutdown, psc);
! 	if (psc->sc_sdhook == NULL) {
  		aprint_error("couldn't make shutdown hook\n");
  		goto bad3;
  	}
***************
*** 228,234 ****
  	if (ath_attach(PCI_PRODUCT(pa->pa_id), sc) == 0)
  		return;
  
! 	shutdownhook_disestablish(shook);
  	powerhook_disestablish(phook);
  
  bad3:	pci_intr_disestablish(pc, psc->sc_ih);
--- 228,234 ----
  	if (ath_attach(PCI_PRODUCT(pa->pa_id), sc) == 0)
  		return;
  
! 	shutdownhook_disestablish(psc->sc_sdhook);
  	powerhook_disestablish(phook);
  
  bad3:	pci_intr_disestablish(pc, psc->sc_ih);
***************
*** 243,248 ****
--- 243,249 ----
  {
  	struct ath_pci_softc *psc = (struct ath_pci_softc *)self;
  
+ 	shutdownhook_disestablish(psc->sc_sdhook);
  	ath_detach(&psc->sc_sc);
  	pci_intr_disestablish(psc->sc_pc, psc->sc_ih);