Date: Thu, 18 Sep 2014 02:01:36 +0000 (UTC) From: Will Andrews <will@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r271731 - head/sys/dev/isp Message-ID: <201409180201.s8I21aHs014235@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: will Date: Thu Sep 18 02:01:36 2014 New Revision: 271731 URL: http://svnweb.freebsd.org/changeset/base/271731 Log: Fix a kernel panic when unloading isp(4). In the current implementation, the isp_kthread() threads never exit. The target threads do have an exit mode from isp_attach(), but it is not invoked from isp_detach(). Ensure isp_detach() notifies threads started for each channel, such that they exit before their parent device softc detaches, and thus before the module does. Otherwise, a page fault panic occurs later in: sysctl_kern_proc sysctl_out_proc kern_proc_out fill_kinfo_proc fill_kinfo_thread strlcpy(kp->ki_wmesg, td->td_wmesg, sizeof(kp->ki_wmesg)); For isp_kthread() (and isp(4) target threads), td->td_wmesg references now-unmapped memory after the module has been unloaded. These threads are typically msleep()ing at the time of unload, but they could also attempt to execute now-unmapped code segments. MFC after: 1 month Sponsored by: Spectra Logic MFSpectraBSD: r1070921 on 2014/06/22 13:01:17 Modified: head/sys/dev/isp/isp_freebsd.c head/sys/dev/isp/isp_freebsd.h Modified: head/sys/dev/isp/isp_freebsd.c ============================================================================== --- head/sys/dev/isp/isp_freebsd.c Thu Sep 18 01:57:36 2014 (r271730) +++ head/sys/dev/isp/isp_freebsd.c Thu Sep 18 02:01:36 2014 (r271731) @@ -132,6 +132,7 @@ isp_attach_chan(ispsoftc_t *isp, struct ISP_SET_PC(isp, chan, proc_active, 0); isp_prt(isp, ISP_LOGERR, "cannot create test target thread"); } + ISP_SPI_PC(isp, chan)->num_threads += 1; #endif } else { fcparam *fcp = FCPARAM(isp, chan); @@ -167,12 +168,14 @@ isp_attach_chan(ispsoftc_t *isp, struct cam_sim_free(fc->sim, FALSE); return (ENOMEM); } + ISP_FC_PC(isp, chan)->num_threads += 1; #ifdef ISP_INTERNAL_TARGET ISP_SET_PC(isp, chan, proc_active, 1); if (THREAD_CREATE(isp_target_thread_fc, fc, &fc->target_proc, 0, 0, "%s: isp_test_tgt%d", device_get_nameunit(isp->isp_osinfo.dev), chan)) { ISP_SET_PC(isp, chan, proc_active, 0); isp_prt(isp, ISP_LOGERR, "cannot create test target thread"); } + ISP_FC_PC(isp, chan)->num_threads += 1; #endif if (chan == 0) { struct sysctl_ctx_list *ctx = device_get_sysctl_ctx(isp->isp_osinfo.dev); @@ -189,6 +192,47 @@ isp_attach_chan(ispsoftc_t *isp, struct return (0); } +static void +isp_detach_internal_target(ispsoftc_t *isp, int chan) +{ +#ifdef ISP_INTERNAL_TARGET + void *wchan; + + ISP_GET_PC(isp, chan, target_proc, wchan); + ISP_SET_PC(isp, chan, proc_active, 0); + wakeup(wchan); +#endif +} + +static void +isp_detach_chan(ispsoftc_t *isp, int chan) +{ + struct cam_sim *sim; + struct cam_path *path; + struct ccb_setasync csa; + int *num_threads; + + ISP_GET_PC(isp, chan, sim, sim); + ISP_GET_PC(isp, chan, path, path); + ISP_GET_PC_ADDR(isp, chan, num_threads, num_threads); + + xpt_setup_ccb(&csa.ccb_h, path, 5); + csa.ccb_h.func_code = XPT_SASYNC_CB; + csa.event_enable = 0; + csa.callback = isp_cam_async; + csa.callback_arg = sim; + xpt_action((union ccb *)&csa); + xpt_free_path(path); + xpt_bus_deregister(cam_sim_path(sim)); + cam_sim_free(sim, FALSE); + + /* Wait for the channel's spawned threads to exit. */ + wakeup(isp->isp_osinfo.pc.ptr); + isp_detach_internal_target(isp, chan); + while (*num_threads != 0) + mtx_sleep(isp, &isp->isp_osinfo.lock, PRIBIO, "isp_reap", 100); +} + int isp_attach(ispsoftc_t *isp) { @@ -239,13 +283,9 @@ unwind: while (--chan >= 0) { struct cam_sim *sim; struct cam_path *path; - if (IS_FC(isp)) { - sim = ISP_FC_PC(isp, chan)->sim; - path = ISP_FC_PC(isp, chan)->path; - } else { - sim = ISP_SPI_PC(isp, chan)->sim; - path = ISP_SPI_PC(isp, chan)->path; - } + + ISP_GET_PC(isp, chan, sim, sim); + ISP_GET_PC(isp, chan, path, path); xpt_free_path(path); ISP_LOCK(isp); xpt_bus_deregister(cam_sim_path(sim)); @@ -269,49 +309,26 @@ int isp_detach(ispsoftc_t *isp) { struct cam_sim *sim; - struct cam_path *path; - struct ccb_setasync csa; int chan; ISP_LOCK(isp); for (chan = isp->isp_nchan - 1; chan >= 0; chan -= 1) { - if (IS_FC(isp)) { - sim = ISP_FC_PC(isp, chan)->sim; - path = ISP_FC_PC(isp, chan)->path; - } else { - sim = ISP_SPI_PC(isp, chan)->sim; - path = ISP_SPI_PC(isp, chan)->path; - } + ISP_GET_PC(isp, chan, sim, sim); if (sim->refcount > 2) { ISP_UNLOCK(isp); return (EBUSY); } } + /* Tell spawned threads that we're exiting. */ + isp->isp_osinfo.is_exiting = 1; if (isp->isp_osinfo.timer_active) { callout_stop(&isp->isp_osinfo.tmo); isp->isp_osinfo.timer_active = 0; } - for (chan = isp->isp_nchan - 1; chan >= 0; chan -= 1) { - if (IS_FC(isp)) { - sim = ISP_FC_PC(isp, chan)->sim; - path = ISP_FC_PC(isp, chan)->path; - } else { - sim = ISP_SPI_PC(isp, chan)->sim; - path = ISP_SPI_PC(isp, chan)->path; - } - xpt_setup_ccb(&csa.ccb_h, path, 5); - csa.ccb_h.func_code = XPT_SASYNC_CB; - csa.event_enable = 0; - csa.callback = isp_cam_async; - csa.callback_arg = sim; - ISP_LOCK(isp); - xpt_action((union ccb *)&csa); - ISP_UNLOCK(isp); - xpt_free_path(path); - xpt_bus_deregister(cam_sim_path(sim)); - cam_sim_free(sim, FALSE); - } + for (chan = isp->isp_nchan - 1; chan >= 0; chan -= 1) + isp_detach_chan(isp, chan); ISP_UNLOCK(isp); + if (isp->isp_osinfo.cdev) { destroy_dev(isp->isp_osinfo.cdev); isp->isp_osinfo.cdev = NULL; @@ -4225,7 +4242,7 @@ isp_target_thread(ispsoftc_t *isp, int c /* * Add resources */ - ISP_GET_PC_ADDR(isp, chan, target_proc, wchan); + ISP_GET_PC(isp, chan, target_proc, wchan); for (i = 0; i < 4; i++) { ccb = malloc(sizeof (*ccb), M_ISPTARG, M_WAITOK | M_ZERO); xpt_setup_ccb(&ccb->ccb_h, wperiph->path, 1); @@ -4313,14 +4330,24 @@ static void isp_target_thread_pi(void *arg) { struct isp_spi *pi = arg; - isp_target_thread(cam_sim_softc(pi->sim), cam_sim_bus(pi->sim)); + ispsoftc_t *isp = cam_sim_softc(pi->sim); + int chan = cam_sim_bus(pi->sim); + + isp_target_thread(isp, chan); + ISP_SPI_PC(isp, chan)->num_threads -= 1; + kthread_exit(); } static void isp_target_thread_fc(void *arg) { struct isp_fc *fc = arg; - isp_target_thread(cam_sim_softc(fc->sim), cam_sim_bus(fc->sim)); + ispsoftc_t *isp = cam_sim_softc(pi->sim); + int chan = cam_sim_bus(pi->sim); + + isp_target_thread(isp, chan); + ISP_FC_PC(isp, chan)->num_threads -= 1; + kthread_exit(); } static int @@ -4748,7 +4775,7 @@ isp_kthread(void *arg) mtx_lock(&isp->isp_osinfo.lock); - for (;;) { + while (isp->isp_osinfo.is_exiting == 0) { int lb, lim; isp_prt(isp, ISP_LOG_SANCFG|ISP_LOGDEBUG0, "%s: Chan %d checking FC state", __func__, chan); @@ -4844,7 +4871,9 @@ isp_kthread(void *arg) mtx_lock(&isp->isp_osinfo.lock); } } + fc->num_threads -= 1; mtx_unlock(&isp->isp_osinfo.lock); + kthread_exit(); } static void Modified: head/sys/dev/isp/isp_freebsd.h ============================================================================== --- head/sys/dev/isp/isp_freebsd.h Thu Sep 18 01:57:36 2014 (r271730) +++ head/sys/dev/isp/isp_freebsd.h Thu Sep 18 02:01:36 2014 (r271731) @@ -268,6 +268,7 @@ struct isp_fc { unsigned int inject_lost_data_frame; #endif #endif + int num_threads; }; struct isp_spi { @@ -291,6 +292,7 @@ struct isp_spi { struct proc * target_proc; #endif #endif + int num_threads; }; struct isposinfo { @@ -365,6 +367,8 @@ struct isposinfo { struct isp_spi *spi; void *ptr; } pc; + + int is_exiting; }; #define ISP_FC_PC(isp, chan) (&(isp)->isp_osinfo.pc.fc[(chan)]) #define ISP_SPI_PC(isp, chan) (&(isp)->isp_osinfo.pc.spi[(chan)])
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201409180201.s8I21aHs014235>