DF-5044 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 | --- a/sys/dev/disk/sili/sili_dragonfly.c +++ b/sys/dev/disk/sili/sili_dragonfly.c @@ -205,22 +205,43 @@ /* * Stop the OS-specific port helper thread and kill the per-port lock. */ -void +int sili_os_stop_port(struct sili_port *ap) { if (ap->ap_thread) { + int t; + sili_os_signal_port_thread(ap, AP_SIGF_STOP); sili_os_sleep(10); if (ap->ap_thread) { kprintf("%s: Waiting for thread to terminate\n", PORTNAME(ap)); - while (ap->ap_thread) - sili_os_sleep(100); + /* + * Bounded wait: a helper thread wedged in the driver + * core (or behind a stuck lock) must not turn + * kldunload/shutdown into a permanent hang. On + * timeout the caller leaks the port instead of + * freeing memory the thread still references. + */ + t = 60; /* seconds */ + while (ap->ap_thread && t > 0) { + sili_os_sleep(1000); + --t; + } + if (ap->ap_thread) { + kprintf("%s: thread did not terminate; " + "leaking port to avoid unload hang\n", + PORTNAME(ap)); + return (ETIMEDOUT); + } kprintf("%s: thread terminated\n", PORTNAME(ap)); } } lockuninit(&ap->ap_lock); + lockuninit(&ap->ap_sim_lock); + lockuninit(&ap->ap_sig_lock); + return (0); } /* --- a/sys/dev/disk/sili/sili.h +++ b/sys/dev/disk/sili/sili.h @@ -869,7 +869,7 @@ int sili_port_init(struct sili_port *ap); int sili_port_alloc(struct sili_softc *, u_int); void sili_port_state_machine(struct sili_port *ap, int initial); -void sili_port_free(struct sili_softc *, u_int); +int sili_port_free(struct sili_softc *, u_int); int sili_port_reset(struct sili_port *, struct ata_port *at, int); void sili_exclusive_access(struct sili_port *ap); @@ -921,7 +921,7 @@ void sili_os_hardsleep(int us); int sili_os_softsleep(void); void sili_os_start_port(struct sili_port *ap); -void sili_os_stop_port(struct sili_port *ap); +int sili_os_stop_port(struct sili_port *ap); void sili_os_signal_port_thread(struct sili_port *ap, int mask); void sili_os_lock_port(struct sili_port *ap); int sili_os_lock_port_nb(struct sili_port *ap); --- a/sys/dev/disk/sili/sili.c +++ b/sys/dev/disk/sili/sili.c @@ -661,7 +661,7 @@ /* * De-initialize and detach a port. */ -void +int sili_port_free(struct sili_softc *sc, u_int port) { struct sili_port *ap = sc->sc_ports[port]; @@ -671,7 +671,15 @@ * Ensure port is disabled and its interrupts are all flushed. */ if (ap->ap_sc) { - sili_os_stop_port(ap); + if (sili_os_stop_port(ap) != 0) { + /* + * Helper thread wedged: keep the port (and the + * controller resources, via the caller) allocated so + * the thread never touches freed memory; leak, do + * not hang. + */ + return (ETIMEDOUT); + } sili_pwrite(ap, SILI_PREG_INT_DISABLE, SILI_PREG_INT_MASK); sili_pwrite(ap, SILI_PREG_CTL_SET, SILI_PREG_CTL_RESET); sili_write(ap->ap_sc, SILI_REG_GCTL, @@ -714,8 +722,10 @@ /* bus_space(9) says we dont free the subregions handle */ + lockuninit(&ap->ap_ccb_lock); kfree(ap, M_DEVBUF); sc->sc_ports[port] = NULL; + return (0); } /* --- a/sys/dev/disk/sili/sili_attach.c +++ b/sys/dev/disk/sili/sili_attach.c @@ -329,6 +329,7 @@ { struct sili_softc *sc = device_get_softc(dev); struct sili_port *ap; + int error = 0; int i; /* @@ -352,9 +353,21 @@ ap = sc->sc_ports[i]; if (ap) { sili_cam_detach(ap); - sili_port_free(sc, i); + if (sili_port_free(sc, i) != 0) + error = EBUSY; } } + if (error) { + /* + * A port helper thread is wedged. Keep every controller + * resource (IRQ/BARs/tags) allocated so the thread never + * touches freed memory, and fail the detach so kldunload + * reports an error instead of hanging the kernel. + */ + device_printf(dev, "port thread stuck; leaking controller " + "resources to avoid unload hang\n"); + return (error); + } /* * Clean up the bus space |