Date: Tue, 13 Aug 2013 16:05:47 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r254284 - in projects/camlock/sys/cam: . ata scsi Message-ID: <201308131605.r7DG5lhi079687@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Tue Aug 13 16:05:47 2013 New Revision: 254284 URL: http://svnweb.freebsd.org/changeset/base/254284 Log: - Introduce new CCB flag CAM_UNLOCKED to allow completion callback to be called without lock held. - Use the flag above for asyncs instead of dropping it inside the handler. - Tune bus scan locking to sinlence WITTNESS warnings and protect state. Modified: projects/camlock/sys/cam/ata/ata_xpt.c projects/camlock/sys/cam/cam_ccb.h projects/camlock/sys/cam/cam_xpt.c projects/camlock/sys/cam/scsi/scsi_xpt.c Modified: projects/camlock/sys/cam/ata/ata_xpt.c ============================================================================== --- projects/camlock/sys/cam/ata/ata_xpt.c Tue Aug 13 15:40:43 2013 (r254283) +++ projects/camlock/sys/cam/ata/ata_xpt.c Tue Aug 13 16:05:47 2013 (r254284) @@ -1321,6 +1321,7 @@ ata_scan_bus(struct cam_periph *periph, struct cam_path *path; ata_scan_bus_info *scan_info; union ccb *work_ccb, *reset_ccb; + struct mtx *mtx; cam_status status; CAM_DEBUG(request_ccb->ccb_h.path, CAM_DEBUG_TRACE, @@ -1396,11 +1397,14 @@ ata_scan_bus(struct cam_periph *periph, xpt_done(request_ccb); break; } + mtx = xpt_path_mtx(scan_info->request_ccb->ccb_h.path); goto scan_next; case XPT_SCAN_LUN: work_ccb = request_ccb; /* Reuse the same CCB to query if a device was really found */ scan_info = (ata_scan_bus_info *)work_ccb->ccb_h.ppriv_ptr0; + mtx = xpt_path_mtx(scan_info->request_ccb->ccb_h.path); + mtx_lock(mtx); /* If there is PMP... */ if ((scan_info->cpi->hba_inquiry & PI_SATAPM) && (scan_info->counter == scan_info->cpi->max_target)) { @@ -1429,6 +1433,7 @@ ata_scan_bus(struct cam_periph *periph, ((scan_info->cpi->hba_inquiry & PI_SATAPM) ? 0 : scan_info->cpi->max_target)) { done: + mtx_unlock(mtx); xpt_free_ccb(work_ccb); xpt_free_ccb((union ccb *)scan_info->cpi); request_ccb = scan_info->request_ccb; @@ -1445,6 +1450,8 @@ scan_next: scan_info->request_ccb->ccb_h.path_id, scan_info->counter, 0); if (status != CAM_REQ_CMP) { + if (request_ccb->ccb_h.func_code == XPT_SCAN_LUN) + mtx_unlock(mtx); printf("xpt_scan_bus: xpt_create_path failed" " with status %#x, bus scan halted\n", status); @@ -1460,9 +1467,15 @@ scan_next: scan_info->request_ccb->ccb_h.pinfo.priority); work_ccb->ccb_h.func_code = XPT_SCAN_LUN; work_ccb->ccb_h.cbfcnp = ata_scan_bus; + work_ccb->ccb_h.flags |= CAM_UNLOCKED; work_ccb->ccb_h.ppriv_ptr0 = scan_info; work_ccb->crcn.flags = scan_info->request_ccb->crcn.flags; + mtx_unlock(mtx); + if (request_ccb->ccb_h.func_code == XPT_SCAN_LUN) + mtx = NULL; xpt_action(work_ccb); + if (mtx != NULL) + mtx_lock(mtx); break; default: break; @@ -1512,6 +1525,7 @@ ata_scan_lun(struct cam_periph *periph, } xpt_setup_ccb(&request_ccb->ccb_h, new_path, CAM_PRIORITY_XPT); request_ccb->ccb_h.cbfcnp = xptscandone; + request_ccb->ccb_h.flags |= CAM_UNLOCKED; request_ccb->ccb_h.func_code = XPT_SCAN_LUN; request_ccb->crcn.flags = flags; } Modified: projects/camlock/sys/cam/cam_ccb.h ============================================================================== --- projects/camlock/sys/cam/cam_ccb.h Tue Aug 13 15:40:43 2013 (r254283) +++ projects/camlock/sys/cam/cam_ccb.h Tue Aug 13 16:05:47 2013 (r254284) @@ -104,7 +104,9 @@ typedef enum { CAM_SEND_SENSE = 0x08000000,/* Send sense data with status */ CAM_TERM_IO = 0x10000000,/* Terminate I/O Message sup. */ CAM_DISCONNECT = 0x20000000,/* Disconnects are mandatory */ - CAM_SEND_STATUS = 0x40000000 /* Send status after data phase */ + CAM_SEND_STATUS = 0x40000000,/* Send status after data phase */ + + CAM_UNLOCKED = 0x80000000 /* Call callback without lock. */ } ccb_flags; /* XPT Opcodes for xpt_action */ Modified: projects/camlock/sys/cam/cam_xpt.c ============================================================================== --- projects/camlock/sys/cam/cam_xpt.c Tue Aug 13 15:40:43 2013 (r254283) +++ projects/camlock/sys/cam/cam_xpt.c Tue Aug 13 16:05:47 2013 (r254284) @@ -770,6 +770,7 @@ static void xpt_scanner_thread(void *dummy) { union ccb *ccb; + struct cam_path path; xpt_lock_buses(); for (;;) { @@ -780,7 +781,16 @@ xpt_scanner_thread(void *dummy) TAILQ_REMOVE(&xsoftc.ccb_scanq, &ccb->ccb_h, sim_links.tqe); xpt_unlock_buses(); + /* + * Since lock can be dropped inside and path freed + * by completion callback even before return here, + * take our own path copy for reference. + */ + xpt_copy_path(&path, ccb->ccb_h.path); + xpt_path_lock(&path); xpt_action(ccb); + xpt_path_unlock(&path); + xpt_release_path(&path); xpt_lock_buses(); } @@ -4034,7 +4044,6 @@ xpt_async_process(struct cam_periph *per u_int32_t async_code; path = ccb->ccb_h.path; - xpt_path_unlock(path); async_code = ccb->casync.async_code; async_arg = ccb->casync.async_arg_ptr; CAM_DEBUG(path, CAM_DEBUG_TRACE | CAM_DEBUG_INFO, @@ -4064,7 +4073,6 @@ xpt_async_process(struct cam_periph *per xpt_release_simq(path->bus->sim, TRUE); if (ccb->casync.async_arg_size > 0) free(async_arg, M_CAMXPT); - xpt_path_lock(path); xpt_free_path(path); xpt_free_ccb(ccb); } @@ -4115,6 +4123,7 @@ xpt_async(u_int32_t async_code, struct c ccb->ccb_h.path->periph = NULL; ccb->ccb_h.func_code = XPT_ASYNC; ccb->ccb_h.cbfcnp = xpt_async_process; + ccb->ccb_h.flags |= CAM_UNLOCKED; ccb->casync.async_code = async_code; ccb->casync.async_arg_size = 0; size = xpt_async_size(async_code); @@ -5169,9 +5178,15 @@ camisr_runqueue(struct cam_sim *sim) ccb_h->status &= ~CAM_DEV_QFRZN; } + if (ccb_h->flags & CAM_UNLOCKED) { + mtx_unlock(mtx); + mtx = NULL; + } + /* Call the peripheral driver's callback */ (*ccb_h->cbfcnp)(ccb_h->path->periph, (union ccb *)ccb_h); - mtx_unlock(mtx); + if (mtx != NULL) + mtx_unlock(mtx); mtx_lock(&sim->sim_doneq_mtx); } sim->sim_doneq_flags &= ~CAM_SIM_DQ_ONQ; Modified: projects/camlock/sys/cam/scsi/scsi_xpt.c ============================================================================== --- projects/camlock/sys/cam/scsi/scsi_xpt.c Tue Aug 13 15:40:43 2013 (r254283) +++ projects/camlock/sys/cam/scsi/scsi_xpt.c Tue Aug 13 16:05:47 2013 (r254284) @@ -1836,6 +1836,8 @@ typedef struct { static void scsi_scan_bus(struct cam_periph *periph, union ccb *request_ccb) { + struct mtx *mtx; + CAM_DEBUG(request_ccb->ccb_h.path, CAM_DEBUG_TRACE, ("scsi_scan_bus\n")); switch (request_ccb->ccb_h.func_code) { @@ -1934,6 +1936,8 @@ scsi_scan_bus(struct cam_periph *periph, scan_info->counter--; } } + mtx = xpt_path_mtx(scan_info->request_ccb->ccb_h.path); + mtx_unlock(mtx); for (i = low_target; i <= max_target; i++) { cam_status status; @@ -1966,10 +1970,13 @@ scsi_scan_bus(struct cam_periph *periph, request_ccb->ccb_h.pinfo.priority); work_ccb->ccb_h.func_code = XPT_SCAN_LUN; work_ccb->ccb_h.cbfcnp = scsi_scan_bus; + work_ccb->ccb_h.flags |= CAM_UNLOCKED; work_ccb->ccb_h.ppriv_ptr0 = scan_info; work_ccb->crcn.flags = request_ccb->crcn.flags; xpt_action(work_ccb); } + + mtx_lock(mtx); break; } case XPT_SCAN_LUN: @@ -2002,6 +2009,8 @@ scsi_scan_bus(struct cam_periph *periph, target = request_ccb->ccb_h.path->target; next_target = 1; + mtx = xpt_path_mtx(scan_info->request_ccb->ccb_h.path); + mtx_lock(mtx); if (target->luns) { uint32_t first; u_int nluns = scsi_4btoul(target->luns->length) / 8; @@ -2125,6 +2134,7 @@ scsi_scan_bus(struct cam_periph *periph, } } if (done) { + mtx_unlock(mtx); xpt_free_ccb(request_ccb); xpt_free_ccb((union ccb *)scan_info->cpi); request_ccb = scan_info->request_ccb; @@ -2138,6 +2148,7 @@ scsi_scan_bus(struct cam_periph *periph, } if ((scan_info->cpi->hba_misc & PIM_SEQSCAN) == 0) { + mtx_unlock(mtx); xpt_free_ccb(request_ccb); break; } @@ -2145,6 +2156,7 @@ scsi_scan_bus(struct cam_periph *periph, scan_info->request_ccb->ccb_h.path_id, scan_info->counter, 0); if (status != CAM_REQ_CMP) { + mtx_unlock(mtx); printf("scsi_scan_bus: xpt_create_path failed" " with status %#x, bus scan halted\n", status); @@ -2160,6 +2172,7 @@ scsi_scan_bus(struct cam_periph *periph, request_ccb->ccb_h.pinfo.priority); request_ccb->ccb_h.func_code = XPT_SCAN_LUN; request_ccb->ccb_h.cbfcnp = scsi_scan_bus; + request_ccb->ccb_h.flags |= CAM_UNLOCKED; request_ccb->ccb_h.ppriv_ptr0 = scan_info; request_ccb->crcn.flags = scan_info->request_ccb->crcn.flags; @@ -2183,10 +2196,12 @@ scsi_scan_bus(struct cam_periph *periph, request_ccb->ccb_h.pinfo.priority); request_ccb->ccb_h.func_code = XPT_SCAN_LUN; request_ccb->ccb_h.cbfcnp = scsi_scan_bus; + request_ccb->ccb_h.flags |= CAM_UNLOCKED; request_ccb->ccb_h.ppriv_ptr0 = scan_info; request_ccb->crcn.flags = scan_info->request_ccb->crcn.flags; } + mtx_unlock(mtx); xpt_action(request_ccb); break; } @@ -2251,6 +2266,7 @@ scsi_scan_lun(struct cam_periph *periph, xpt_setup_ccb(&request_ccb->ccb_h, new_path, CAM_PRIORITY_XPT); request_ccb->ccb_h.cbfcnp = xptscandone; request_ccb->ccb_h.func_code = XPT_SCAN_LUN; + request_ccb->ccb_h.flags |= CAM_UNLOCKED; request_ccb->crcn.flags = flags; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308131605.r7DG5lhi079687>