Date: Wed, 1 Nov 2006 00:45:27 GMT From: Scott Long <scottl@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 108876 for review Message-ID: <200611010045.kA10jRKw053035@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=108876 Change 108876 by scottl@scottl-x64 on 2006/11/01 00:44:27 Don't hold the MPT lock over all of mpt_attach(), it causes all sorts of problems with memory allocations and system APIs. Affected files ... .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt.c#14 edit .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_cam.c#15 edit .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_pci.c#16 edit .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_raid.c#9 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt.c#14 (text+ko) ==== @@ -2110,11 +2110,13 @@ TAILQ_INIT(&mpt->request_pending_list); TAILQ_INIT(&mpt->request_free_list); TAILQ_INIT(&mpt->request_timeout_list); + MPT_LOCK(mpt); for (val = 0; val < MPT_MAX_REQUESTS(mpt); val++) { request_t *req = &mpt->request_pool[val]; req->state = REQ_STATE_ALLOCATED; mpt_free_request(mpt, req); } + MPT_UNLOCK(mpt); for (val = 0; val < MPT_MAX_LUNS; val++) { STAILQ_INIT(&mpt->trt[val].atios); @@ -2130,7 +2132,9 @@ mpt_lprt(mpt, MPT_PRT_DEBUG, "doorbell req = %s\n", mpt_ioc_diag(mpt_read(mpt, MPT_OFFSET_DOORBELL))); + MPT_LOCK(mpt); error = mpt_configure_ioc(mpt); + MPT_UNLOCK(mpt); return (error); } @@ -2143,6 +2147,7 @@ * not enabled, ports not enabled and interrupts * not enabled. */ + MPT_LOCK(mpt); /* * Enable asynchronous event reporting- all personalities @@ -2177,8 +2182,10 @@ */ if (mpt_send_port_enable(mpt, 0) != MPT_OK) { mpt_prt(mpt, "failed to enable port 0\n"); + MPT_UNLOCK(mpt); return (ENXIO); } + MPT_UNLOCK(mpt); return (0); } ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_cam.c#15 (text+ko) ==== @@ -210,6 +210,7 @@ int maxq; int error; + MPT_LOCK(mpt); TAILQ_INIT(&mpt->request_timeout_list); maxq = (mpt->mpt_global_credits < MPT_MAX_REQUESTS(mpt))? mpt->mpt_global_credits : MPT_MAX_REQUESTS(mpt); @@ -218,14 +219,16 @@ error = mpt_register_handler(mpt, MPT_HANDLER_REPLY, handler, &scsi_io_handler_id); if (error != 0) { - goto cleanup0; + MPT_UNLOCK(mpt); + goto cleanup; } handler.reply_handler = mpt_scsi_tmf_reply_handler; error = mpt_register_handler(mpt, MPT_HANDLER_REPLY, handler, &scsi_tmf_handler_id); if (error != 0) { - goto cleanup0; + MPT_UNLOCK(mpt); + goto cleanup; } /* @@ -237,11 +240,13 @@ error = mpt_register_handler(mpt, MPT_HANDLER_REPLY, handler, &fc_els_handler_id); if (error != 0) { - goto cleanup0; + MPT_UNLOCK(mpt); + goto cleanup; } if (mpt_add_els_buffers(mpt) == FALSE) { error = ENOMEM; - goto cleanup0; + MPT_UNLOCK(mpt); + goto cleanup; } maxq -= mpt->els_cmds_allocated; } @@ -256,7 +261,8 @@ error = mpt_register_handler(mpt, MPT_HANDLER_REPLY, handler, &mpt->scsi_tgt_handler_id); if (error != 0) { - goto cleanup0; + MPT_UNLOCK(mpt); + goto cleanup; } } @@ -267,7 +273,8 @@ if (mpt->tmf_req == NULL) { mpt_prt(mpt, "Unable to allocate dedicated TMF request!\n"); error = ENOMEM; - goto cleanup0; + MPT_UNLOCK(mpt); + goto cleanup; } /* @@ -279,18 +286,18 @@ mpt->tmf_req->state = REQ_STATE_FREE; maxq--; + /* + * The rest of this is CAM foo, for which we need to drop our lock + */ + MPT_UNLOCK(mpt); + if (mpt_spawn_recovery_thread(mpt) != 0) { mpt_prt(mpt, "Unable to spawn recovery thread!\n"); error = ENOMEM; - goto cleanup0; + goto cleanup; } /* - * The rest of this is CAM foo, for which we need to drop our lock - */ - MPTLOCK_2_CAMLOCK(mpt); - - /* * Create the device queue for our SIM(s). */ devq = cam_simq_alloc(maxq); @@ -315,9 +322,11 @@ /* * Register exactly this bus. */ + MPT_LOCK(mpt); if (xpt_bus_register(mpt->sim, 0) != CAM_SUCCESS) { mpt_prt(mpt, "Bus registration Failed!\n"); error = ENOMEM; + MPT_UNLOCK(mpt); goto cleanup; } @@ -325,15 +334,16 @@ CAM_TARGET_WILDCARD, CAM_LUN_WILDCARD) != CAM_REQ_CMP) { mpt_prt(mpt, "Unable to allocate Path!\n"); error = ENOMEM; + MPT_UNLOCK(mpt); goto cleanup; } + MPT_UNLOCK(mpt); /* * Only register a second bus for RAID physical * devices if the controller supports RAID. */ if (mpt->ioc_page2 == NULL || mpt->ioc_page2->MaxPhysDisks == 0) { - CAMLOCK_2_MPTLOCK(mpt); return (0); } @@ -351,9 +361,11 @@ /* * Register this bus. */ + MPT_LOCK(mpt); if (xpt_bus_register(mpt->phydisk_sim, 1) != CAM_SUCCESS) { mpt_prt(mpt, "Physical Disk Bus registration Failed!\n"); error = ENOMEM; + MPT_UNLOCK(mpt); goto cleanup; } @@ -362,15 +374,14 @@ CAM_TARGET_WILDCARD, CAM_LUN_WILDCARD) != CAM_REQ_CMP) { mpt_prt(mpt, "Unable to allocate Physical Disk Path!\n"); error = ENOMEM; + MPT_UNLOCK(mpt); goto cleanup; } - CAMLOCK_2_MPTLOCK(mpt); + MPT_UNLOCK(mpt); mpt_lprt(mpt, MPT_PRT_DEBUG, "attached cam\n"); return (0); cleanup: - CAMLOCK_2_MPTLOCK(mpt); -cleanup0: mpt_cam_detach(mpt); return (error); } @@ -793,29 +804,38 @@ int mpt_cam_enable(struct mpt_softc *mpt) { + int error; + + MPT_LOCK(mpt); + + error = EIO; if (mpt->is_fc) { if (mpt_read_config_info_fc(mpt)) { - return (EIO); + goto out; } if (mpt_set_initial_config_fc(mpt)) { - return (EIO); + goto out; } } else if (mpt->is_sas) { if (mpt_read_config_info_sas(mpt)) { - return (EIO); + goto out; } if (mpt_set_initial_config_sas(mpt)) { - return (EIO); + goto out; } } else if (mpt->is_spi) { if (mpt_read_config_info_spi(mpt)) { - return (EIO); + goto out; } if (mpt_set_initial_config_spi(mpt)) { - return (EIO); + goto out; } } - return (0); + error = 0; + +out: + MPT_UNLOCK(mpt); + return (error); } void @@ -842,6 +862,7 @@ { mpt_handler_t handler; + MPT_LOCK(mpt); mpt_terminate_recovery_thread(mpt); handler.reply_handler = mpt_scsi_reply_handler; @@ -862,23 +883,20 @@ mpt_free_request(mpt, mpt->tmf_req); mpt->tmf_req = NULL; } + MPT_UNLOCK(mpt); if (mpt->sim != NULL) { - MPTLOCK_2_CAMLOCK(mpt); xpt_free_path(mpt->path); xpt_bus_deregister(cam_sim_path(mpt->sim)); cam_sim_free(mpt->sim, TRUE); mpt->sim = NULL; - CAMLOCK_2_MPTLOCK(mpt); } if (mpt->phydisk_sim != NULL) { - MPTLOCK_2_CAMLOCK(mpt); xpt_free_path(mpt->phydisk_path); xpt_bus_deregister(cam_sim_path(mpt->phydisk_sim)); cam_sim_free(mpt->phydisk_sim, TRUE); mpt->phydisk_sim = NULL; - CAMLOCK_2_MPTLOCK(mpt); } } ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_pci.c#16 (text+ko) ==== @@ -559,12 +559,9 @@ /* Initialize the hardware */ if (mpt->disabled == 0) { - MPT_LOCK(mpt); if (mpt_attach(mpt) != 0) { - MPT_UNLOCK(mpt); goto bad; } - MPT_UNLOCK(mpt); } else { mpt_prt(mpt, "device disabled at user request\n"); goto bad; @@ -575,9 +572,7 @@ if (mpt->eh == NULL) { mpt_prt(mpt, "shutdown event registration failed\n"); - MPT_LOCK(mpt); (void) mpt_detach(mpt); - MPT_UNLOCK(mpt); goto bad; } return (0); @@ -636,7 +631,6 @@ mpt = (struct mpt_softc*)device_get_softc(dev); if (mpt) { - MPT_LOCK(mpt); mpt_disable_ints(mpt); mpt_detach(mpt); mpt_reset(mpt, /*reinit*/FALSE); @@ -646,7 +640,6 @@ if (mpt->eh != NULL) { EVENTHANDLER_DEREGISTER(shutdown_final, mpt->eh); } - MPT_UNLOCK(mpt); } return(0); } @@ -663,9 +656,7 @@ mpt = (struct mpt_softc *)device_get_softc(dev); if (mpt) { int r; - MPT_LOCK(mpt); r = mpt_shutdown(mpt); - MPT_UNLOCK(mpt); return (r); } return(0); ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_raid.c#9 (text+ko) ==== @@ -270,6 +270,13 @@ mpt_callout_init(&mpt->raid_timer); + error = mpt_spawn_raid_thread(mpt); + if (error != 0) { + mpt_prt(mpt, "Unable to spawn RAID thread!\n"); + goto cleanup; + } + + MPT_LOCK(mpt); handler.reply_handler = mpt_raid_reply_handler; error = mpt_register_handler(mpt, MPT_HANDLER_REPLY, handler, &raid_handler_id); @@ -278,28 +285,22 @@ goto cleanup; } - error = mpt_spawn_raid_thread(mpt); - if (error != 0) { - mpt_prt(mpt, "Unable to spawn RAID thread!\n"); - goto cleanup; - } - xpt_setup_ccb(&csa.ccb_h, mpt->path, 5); csa.ccb_h.func_code = XPT_SASYNC_CB; csa.event_enable = AC_FOUND_DEVICE; csa.callback = mpt_raid_async; csa.callback_arg = mpt; - MPTLOCK_2_CAMLOCK(mpt); xpt_action((union ccb *)&csa); - CAMLOCK_2_MPTLOCK(mpt); if (csa.ccb_h.status != CAM_REQ_CMP) { mpt_prt(mpt, "mpt_raid_attach: Unable to register " "CAM async handler.\n"); } + MPT_UNLOCK(mpt); mpt_raid_sysctl_attach(mpt); return (0); cleanup: + MPT_UNLOCK(mpt); mpt_raid_detach(mpt); return (error); } @@ -317,6 +318,7 @@ mpt_handler_t handler; callout_stop(&mpt->raid_timer); + MPT_LOCK(mpt); mpt_terminate_raid_thread(mpt); handler.reply_handler = mpt_raid_reply_handler; @@ -327,9 +329,8 @@ csa.event_enable = 0; csa.callback = mpt_raid_async; csa.callback_arg = mpt; - MPTLOCK_2_CAMLOCK(mpt); xpt_action((union ccb *)&csa); - CAMLOCK_2_MPTLOCK(mpt); + MPT_UNLOCK(mpt); } static void @@ -620,12 +621,17 @@ * reject I/O to an ID we later determine is for a * hidden physdisk. */ + MPT_LOCK(mpt); xpt_freeze_simq(mpt->phydisk_sim, 1); + MPT_UNLOCK(mpt); error = mpt_kthread_create(mpt_raid_thread, mpt, &mpt->raid_thread, /*flags*/0, /*altstack*/0, "mpt_raid%d", mpt->unit); - if (error != 0) + if (error != 0) { + MPT_LOCK(mpt); xpt_release_simq(mpt->phydisk_sim, /*run_queue*/FALSE); + MPT_UNLOCK(mpt); + } return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200611010045.kA10jRKw053035>