Date: Wed, 2 Dec 2009 10:10:37 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r200021 - in stable/8/sys/cam: . scsi Message-ID: <200912021010.nB2AAbCb094399@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Wed Dec 2 10:10:37 2009 New Revision: 200021 URL: http://svn.freebsd.org/changeset/base/200021 Log: MFC r199279, r199280, r199281: - Fix several device freeze counting bugs. - Remove code that years ago was closing race between request submission to SIM and device/SIM freeze. That race become impossible after moving from spl to mutex locking, while this workaround causes some unexpected effects. Modified: stable/8/sys/cam/cam_periph.c stable/8/sys/cam/cam_queue.c stable/8/sys/cam/cam_queue.h stable/8/sys/cam/cam_xpt.c stable/8/sys/cam/scsi/scsi_cd.c stable/8/sys/cam/scsi/scsi_ch.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) Modified: stable/8/sys/cam/cam_periph.c ============================================================================== --- stable/8/sys/cam/cam_periph.c Wed Dec 2 08:52:06 2009 (r200020) +++ stable/8/sys/cam/cam_periph.c Wed Dec 2 10:10:37 2009 (r200021) @@ -981,16 +981,21 @@ camperiphdone(struct cam_periph *periph, { union ccb *saved_ccb; cam_status status; - int frozen; + int frozen = 0; int sense; struct scsi_start_stop_unit *scsi_cmd; u_int32_t relsim_flags, timeout; - u_int32_t qfrozen_cnt; - int xpt_done_ccb; + int xpt_done_ccb = FALSE; - xpt_done_ccb = FALSE; status = done_ccb->ccb_h.status; - frozen = (status & CAM_DEV_QFRZN) != 0; + if (status & CAM_DEV_QFRZN) { + frozen = 1; + /* + * Clear freeze flag now for case of retry, + * freeze will be dropped later. + */ + done_ccb->ccb_h.status &= ~CAM_DEV_QFRZN; + } sense = (status & CAM_AUTOSNS_VALID) != 0; status &= CAM_STATUS_MASK; @@ -998,17 +1003,6 @@ camperiphdone(struct cam_periph *periph, relsim_flags = 0; saved_ccb = (union ccb *)done_ccb->ccb_h.saved_ccb_ptr; - /* - * Unfreeze the queue once if it is already frozen.. - */ - if (frozen != 0) { - qfrozen_cnt = cam_release_devq(done_ccb->ccb_h.path, - /*relsim_flags*/0, - /*openings*/0, - /*timeout*/0, - /*getcount_only*/0); - } - switch (status) { case CAM_REQ_CMP: { @@ -1185,14 +1179,33 @@ camperiphdone(struct cam_periph *periph, */ if (done_ccb->ccb_h.retry_count > 0) done_ccb->ccb_h.retry_count--; - - qfrozen_cnt = cam_release_devq(done_ccb->ccb_h.path, - /*relsim_flags*/relsim_flags, - /*openings*/0, - /*timeout*/timeout, - /*getcount_only*/0); - if (xpt_done_ccb == TRUE) + /* + * Drop freeze taken due to CAM_DEV_QFREEZE flag set on recovery + * request. + */ + cam_release_devq(done_ccb->ccb_h.path, + /*relsim_flags*/relsim_flags, + /*openings*/0, + /*timeout*/timeout, + /*getcount_only*/0); + if (xpt_done_ccb == TRUE) { + /* + * Copy frozen flag from recovery request if it is set there + * for some reason. + */ + if (frozen != 0) + done_ccb->ccb_h.status |= CAM_DEV_QFRZN; (*done_ccb->ccb_h.cbfcnp)(periph, done_ccb); + } else { + /* Drop freeze taken, if this recovery request got error. */ + if (frozen != 0) { + cam_release_devq(done_ccb->ccb_h.path, + /*relsim_flags*/0, + /*openings*/0, + /*timeout*/0, + /*getcount_only*/0); + } + } } /* @@ -1452,6 +1465,11 @@ camperiphscsisenseerror(union ccb *ccb, action_string = "No recovery CCB supplied"; goto sense_error_done; } + /* + * Clear freeze flag for original request here, as + * this freeze will be dropped as part of ERESTART. + */ + ccb->ccb_h.status &= ~CAM_DEV_QFRZN; bcopy(ccb, save_ccb, sizeof(*save_ccb)); print_ccb = save_ccb; periph->flags |= CAM_PERIPH_RECOVERY_INPROG; Modified: stable/8/sys/cam/cam_queue.c ============================================================================== --- stable/8/sys/cam/cam_queue.c Wed Dec 2 08:52:06 2009 (r200020) +++ stable/8/sys/cam/cam_queue.c Wed Dec 2 10:10:37 2009 (r200021) @@ -334,7 +334,6 @@ cam_ccbq_init(struct cam_ccbq *ccbq, int } ccbq->devq_openings = openings; ccbq->dev_openings = openings; - TAILQ_INIT(&ccbq->active_ccbs); return (0); } Modified: stable/8/sys/cam/cam_queue.h ============================================================================== --- stable/8/sys/cam/cam_queue.h Wed Dec 2 08:52:06 2009 (r200020) +++ stable/8/sys/cam/cam_queue.h Wed Dec 2 10:10:37 2009 (r200021) @@ -60,7 +60,6 @@ struct cam_ccbq { int dev_openings; int dev_active; int held; - struct ccb_hdr_tailq active_ccbs; }; struct cam_ed; @@ -209,9 +208,6 @@ static __inline void cam_ccbq_send_ccb(struct cam_ccbq *ccbq, union ccb *send_ccb) { - TAILQ_INSERT_TAIL(&ccbq->active_ccbs, - &(send_ccb->ccb_h), - xpt_links.tqe); send_ccb->ccb_h.pinfo.index = CAM_ACTIVE_INDEX; ccbq->dev_active++; ccbq->dev_openings--; @@ -220,8 +216,7 @@ cam_ccbq_send_ccb(struct cam_ccbq *ccbq, static __inline void cam_ccbq_ccb_done(struct cam_ccbq *ccbq, union ccb *done_ccb) { - TAILQ_REMOVE(&ccbq->active_ccbs, &done_ccb->ccb_h, - xpt_links.tqe); + ccbq->dev_active--; ccbq->dev_openings++; ccbq->held++; Modified: stable/8/sys/cam/cam_xpt.c ============================================================================== --- stable/8/sys/cam/cam_xpt.c Wed Dec 2 08:52:06 2009 (r200020) +++ stable/8/sys/cam/cam_xpt.c Wed Dec 2 10:10:37 2009 (r200021) @@ -3273,16 +3273,13 @@ xpt_run_dev_sendq(struct cam_eb *bus) devq->send_queue.qfrozen_cnt++; while ((devq->send_queue.entries > 0) - && (devq->send_openings > 0)) { + && (devq->send_openings > 0) + && (devq->send_queue.qfrozen_cnt <= 1)) { struct cam_ed_qinfo *qinfo; struct cam_ed *device; union ccb *work_ccb; struct cam_sim *sim; - if (devq->send_queue.qfrozen_cnt > 1) { - break; - } - qinfo = (struct cam_ed_qinfo *)camq_remove(&devq->send_queue, CAMQ_HEAD); device = qinfo->device; @@ -3330,9 +3327,7 @@ xpt_run_dev_sendq(struct cam_eb *bus) } mtx_unlock(&xsoftc.xpt_lock); } - devq->active_dev = device; cam_ccbq_remove_ccb(&device->ccbq, work_ccb); - cam_ccbq_send_ccb(&device->ccbq, work_ccb); devq->send_openings--; @@ -3370,8 +3365,6 @@ xpt_run_dev_sendq(struct cam_eb *bus) */ sim = work_ccb->ccb_h.path->bus->sim; (*(sim->sim_action))(sim, work_ccb); - - devq->active_dev = NULL; } devq->send_queue.qfrozen_cnt--; } @@ -4102,45 +4095,18 @@ xpt_dev_async_default(u_int32_t async_co u_int32_t xpt_freeze_devq(struct cam_path *path, u_int count) { - struct ccb_hdr *ccbh; mtx_assert(path->bus->sim->mtx, MA_OWNED); - path->device->ccbq.queue.qfrozen_cnt += count; - - /* - * Mark the last CCB in the queue as needing - * to be requeued if the driver hasn't - * changed it's state yet. This fixes a race - * where a ccb is just about to be queued to - * a controller driver when it's interrupt routine - * freezes the queue. To completly close the - * hole, controller drives must check to see - * if a ccb's status is still CAM_REQ_INPROG - * just before they queue - * the CCB. See ahc_action/ahc_freeze_devq for - * an example. - */ - ccbh = TAILQ_LAST(&path->device->ccbq.active_ccbs, ccb_hdr_tailq); - if (ccbh && ccbh->status == CAM_REQ_INPROG) - ccbh->status = CAM_REQUEUE_REQ; return (path->device->ccbq.queue.qfrozen_cnt); } u_int32_t xpt_freeze_simq(struct cam_sim *sim, u_int count) { - mtx_assert(sim->mtx, MA_OWNED); + mtx_assert(sim->mtx, MA_OWNED); sim->devq->send_queue.qfrozen_cnt += count; - if (sim->devq->active_dev != NULL) { - struct ccb_hdr *ccbh; - - ccbh = TAILQ_LAST(&sim->devq->active_dev->ccbq.active_ccbs, - ccb_hdr_tailq); - if (ccbh && ccbh->status == CAM_REQ_INPROG) - ccbh->status = CAM_REQUEUE_REQ; - } return (sim->devq->send_queue.qfrozen_cnt); } Modified: stable/8/sys/cam/scsi/scsi_cd.c ============================================================================== --- stable/8/sys/cam/scsi/scsi_cd.c Wed Dec 2 08:52:06 2009 (r200020) +++ stable/8/sys/cam/scsi/scsi_cd.c Wed Dec 2 10:10:37 2009 (r200021) @@ -1570,7 +1570,8 @@ cddone(struct cam_periph *periph, union bp->bio_resid = bp->bio_bcount; bp->bio_error = error; bp->bio_flags |= BIO_ERROR; - cam_release_devq(done_ccb->ccb_h.path, + if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) + cam_release_devq(done_ccb->ccb_h.path, /*relsim_flags*/0, /*reduction*/0, /*timeout*/0, @@ -1658,7 +1659,8 @@ cddone(struct cam_periph *periph, union struct ccb_getdev cgd; /* Don't wedge this device's queue */ - cam_release_devq(done_ccb->ccb_h.path, + if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) + cam_release_devq(done_ccb->ccb_h.path, /*relsim_flags*/0, /*reduction*/0, /*timeout*/0, Modified: stable/8/sys/cam/scsi/scsi_ch.c ============================================================================== --- stable/8/sys/cam/scsi/scsi_ch.c Wed Dec 2 08:52:06 2009 (r200020) +++ stable/8/sys/cam/scsi/scsi_ch.c Wed Dec 2 10:10:37 2009 (r200021) @@ -606,7 +606,8 @@ chdone(struct cam_periph *periph, union retry_scheduled = 0; /* Don't wedge this device's queue */ - cam_release_devq(done_ccb->ccb_h.path, + if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) + cam_release_devq(done_ccb->ccb_h.path, /*relsim_flags*/0, /*reduction*/0, /*timeout*/0,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200912021010.nB2AAbCb094399>