Date: Sun, 10 Nov 2013 12:16:09 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r257914 - head/sys/cam Message-ID: <201311101216.rAACG9Xc056510@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Sun Nov 10 12:16:09 2013 New Revision: 257914 URL: http://svnweb.freebsd.org/changeset/base/257914 Log: Some CAM locks polishing: - Fix LOR and possible lock recursion when handling high-power commands. Introduce new lock to protect left power quota and list of frozen devices. - Correct locking around xpt periph creation. - Remove seems never used XPT_FLAG_OPEN xpt periph flag. Modified: head/sys/cam/cam_xpt.c Modified: head/sys/cam/cam_xpt.c ============================================================================== --- head/sys/cam/cam_xpt.c Sun Nov 10 09:36:51 2013 (r257913) +++ head/sys/cam/cam_xpt.c Sun Nov 10 12:16:09 2013 (r257914) @@ -92,14 +92,9 @@ struct xpt_task { uintptr_t data2; }; -typedef enum { - XPT_FLAG_OPEN = 0x01 -} xpt_flags; - struct xpt_softc { - xpt_flags flags; - /* number of high powered commands that can go through right now */ + struct mtx xpt_highpower_lock; STAILQ_HEAD(highpowerlist, cam_ed) highpowerq; int num_highpower; @@ -240,6 +235,7 @@ static timeout_t xpt_release_devq_timeou static void xpt_release_simq_timeout(void *arg) __unused; static void xpt_acquire_bus(struct cam_eb *bus); static void xpt_release_bus(struct cam_eb *bus); +static uint32_t xpt_freeze_devq_device(struct cam_ed *dev, u_int count); static int xpt_release_devq_device(struct cam_ed *dev, u_int count, int run_queue); static struct cam_et* @@ -367,11 +363,6 @@ xptopen(struct cdev *dev, int flags, int return(ENODEV); } - /* Mark ourselves open */ - mtx_lock(&xsoftc.xpt_lock); - xsoftc.flags |= XPT_FLAG_OPEN; - mtx_unlock(&xsoftc.xpt_lock); - return(0); } @@ -379,11 +370,6 @@ static int xptclose(struct cdev *dev, int flag, int fmt, struct thread *td) { - /* Mark ourselves closed */ - mtx_lock(&xsoftc.xpt_lock); - xsoftc.flags &= ~XPT_FLAG_OPEN; - mtx_unlock(&xsoftc.xpt_lock); - return(0); } @@ -863,6 +849,7 @@ xpt_init(void *dummy) xsoftc.num_highpower = CAM_MAX_HIGHPOWER; mtx_init(&xsoftc.xpt_lock, "XPT lock", NULL, MTX_DEF); + mtx_init(&xsoftc.xpt_highpower_lock, "XPT highpower lock", NULL, MTX_DEF); mtx_init(&xsoftc.xpt_topo_lock, "XPT topology lock", NULL, MTX_DEF); xsoftc.xpt_taskq = taskqueue_create("CAM XPT task", M_WAITOK, taskqueue_thread_enqueue, /*context*/&xsoftc.xpt_taskq); @@ -900,6 +887,7 @@ xpt_init(void *dummy) " failing attach\n", status); return (EINVAL); } + mtx_unlock(&xsoftc.xpt_lock); /* * Looking at the XPT from the SIM layer, the XPT is @@ -914,11 +902,12 @@ xpt_init(void *dummy) " failing attach\n", status); return (EINVAL); } - + xpt_path_lock(path); cam_periph_alloc(xptregister, NULL, NULL, NULL, "xpt", CAM_PERIPH_BIO, path, NULL, 0, xpt_sim); + xpt_path_unlock(path); xpt_free_path(path); - mtx_unlock(&xsoftc.xpt_lock); + if (cam_num_doneqs < 1) cam_num_doneqs = 1 + mp_ncpus / 6; else if (cam_num_doneqs > MAXCPU) @@ -3223,7 +3212,7 @@ xpt_run_devq(struct cam_devq *devq) if ((work_ccb->ccb_h.flags & CAM_HIGH_POWER) != 0) { - mtx_lock(&xsoftc.xpt_lock); + mtx_lock(&xsoftc.xpt_highpower_lock); if (xsoftc.num_highpower <= 0) { /* * We got a high power command, but we @@ -3231,11 +3220,11 @@ xpt_run_devq(struct cam_devq *devq) * the device queue until we have a slot * available. */ - xpt_freeze_devq(work_ccb->ccb_h.path, 1); + xpt_freeze_devq_device(device, 1); STAILQ_INSERT_TAIL(&xsoftc.highpowerq, device, highpowerq_entry); - mtx_unlock(&xsoftc.xpt_lock); + mtx_unlock(&xsoftc.xpt_highpower_lock); continue; } else { /* @@ -3244,7 +3233,7 @@ xpt_run_devq(struct cam_devq *devq) */ xsoftc.num_highpower--; } - mtx_unlock(&xsoftc.xpt_lock); + mtx_unlock(&xsoftc.xpt_highpower_lock); } cam_ccbq_remove_ccb(&device->ccbq, work_ccb); cam_ccbq_send_ccb(&device->ccbq, work_ccb); @@ -4286,21 +4275,35 @@ xpt_dev_async_default(u_int32_t async_co printf("%s called\n", __func__); } -u_int32_t -xpt_freeze_devq(struct cam_path *path, u_int count) +static uint32_t +xpt_freeze_devq_device(struct cam_ed *dev, u_int count) { - struct cam_ed *dev = path->device; struct cam_devq *devq; - uint32_t freeze; + uint32_t freeze; devq = dev->sim->devq; - mtx_lock(&devq->send_mtx); - CAM_DEBUG(path, CAM_DEBUG_TRACE, ("xpt_freeze_devq() %u->%u\n", + mtx_assert(&devq->send_mtx, MA_OWNED); + CAM_DEBUG_DEV(dev, CAM_DEBUG_TRACE, + ("xpt_freeze_devq_device(%d) %u->%u\n", count, dev->ccbq.queue.qfrozen_cnt, dev->ccbq.queue.qfrozen_cnt + count)); freeze = (dev->ccbq.queue.qfrozen_cnt += count); /* Remove frozen device from sendq. */ if (device_is_queued(dev)) camq_remove(&devq->send_queue, dev->devq_entry.index); + return (freeze); +} + +u_int32_t +xpt_freeze_devq(struct cam_path *path, u_int count) +{ + struct cam_ed *dev = path->device; + struct cam_devq *devq; + uint32_t freeze; + + devq = dev->sim->devq; + mtx_lock(&devq->send_mtx); + CAM_DEBUG(path, CAM_DEBUG_TRACE, ("xpt_freeze_devq(%d)\n", count)); + freeze = xpt_freeze_devq_device(dev, count); mtx_unlock(&devq->send_mtx); return (freeze); } @@ -5150,7 +5153,7 @@ xpt_done_process(struct ccb_hdr *ccb_h) struct highpowerlist *hphead; struct cam_ed *device; - mtx_lock(&xsoftc.xpt_lock); + mtx_lock(&xsoftc.xpt_highpower_lock); hphead = &xsoftc.highpowerq; device = STAILQ_FIRST(hphead); @@ -5166,14 +5169,14 @@ xpt_done_process(struct ccb_hdr *ccb_h) if (device != NULL) { STAILQ_REMOVE_HEAD(hphead, highpowerq_entry); - mtx_unlock(&xsoftc.xpt_lock); + mtx_unlock(&xsoftc.xpt_highpower_lock); mtx_lock(&device->sim->devq->send_mtx); xpt_release_devq_device(device, /*count*/1, /*runqueue*/TRUE); mtx_unlock(&device->sim->devq->send_mtx); } else - mtx_unlock(&xsoftc.xpt_lock); + mtx_unlock(&xsoftc.xpt_highpower_lock); } sim = ccb_h->path->bus->sim;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201311101216.rAACG9Xc056510>