Date: Thu, 22 Aug 2013 16:42:41 +0400 (MSK) From: Dmitry Morozovsky <marck@rinet.ru> To: "Kenneth D. Merry" <ken@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r254615 - head/sys/dev/mps Message-ID: <alpine.BSF.2.00.1308221641010.10197@woozle.rinet.ru> In-Reply-To: <201308212130.r7LLUvO5008991@svn.freebsd.org> References: <201308212130.r7LLUvO5008991@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Ken, On Wed, 21 Aug 2013, Kenneth D. Merry wrote: > Author: ken > Date: Wed Aug 21 21:30:56 2013 > New Revision: 254615 > URL: http://svnweb.freebsd.org/changeset/base/254615 > > Log: > Fix mps(4) driver breakage that came in in change 253550 that > manifested itself in out of chain frame conditions. > > When the driver ran out of chain frames, the request in question > would get completed early, and go through mpssas_scsiio_complete(). > > In mpssas_scsiio_complete(), the negation of the CAM status values > (CAM_STATUS_MASK | CAM_SIM_QUEUED) was ORed in instead of being > ANDed in. This resulted in a bogus CAM CCB status value. This > didn't show up in the non-error case, because the status was reset > to something valid (e.g. CAM_REQ_CMP) later on in the function. > > But in the error case, such as when the driver ran out of chain > frames, the CAM_REQUEUE_REQ status was ORed in to the bogus status > value. This led to the CAM transport layer repeatedly releasing > the SIM queue, because it though that the CAM_RELEASE_SIMQ flag had > been set. The symptom was messages like this on the console when > INVARIANTS were enabled: > > xpt_release_simq: requested 1 > present 0 > xpt_release_simq: requested 1 > present 0 > xpt_release_simq: requested 1 > present 0 what is real impact of the bug? > > mps_sas.c: In mpssas_scsiio_complete(), use &= to take status > bits out. |= adds them in. > > In the error case in mpssas_scsiio_complete(), set > the status to CAM_REQUEUE_REQ, don't OR it in. > > MFC after: 3 days This patch does not apply cleanly as r253550 had not been merged, and the first masking does not occur on contemporary stable/9. Comments? Thank you! > Sponsored by: Spectra Logic > > Modified: > head/sys/dev/mps/mps_sas.c > > Modified: head/sys/dev/mps/mps_sas.c > ============================================================================== > --- head/sys/dev/mps/mps_sas.c Wed Aug 21 21:30:06 2013 (r254614) > +++ head/sys/dev/mps/mps_sas.c Wed Aug 21 21:30:56 2013 (r254615) > @@ -2103,7 +2103,7 @@ mpssas_scsiio_complete(struct mps_softc > cm->cm_targ->completed++; > cm->cm_targ->outstanding--; > TAILQ_REMOVE(&cm->cm_targ->commands, cm, cm_link); > - ccb->ccb_h.status |= ~(CAM_STATUS_MASK | CAM_SIM_QUEUED); > + ccb->ccb_h.status &= ~(CAM_STATUS_MASK | CAM_SIM_QUEUED); > > if (cm->cm_state == MPS_CM_STATE_TIMEDOUT) { > TAILQ_REMOVE(&cm->cm_targ->timedout_commands, cm, cm_recovery); > @@ -2145,7 +2145,7 @@ mpssas_scsiio_complete(struct mps_softc > * because there can be no reply when we haven't actually > * gone out to the hardware. > */ > - ccb->ccb_h.status |= CAM_REQUEUE_REQ; > + ccb->ccb_h.status = CAM_REQUEUE_REQ; > > /* > * Currently the only error included in the mask is > _______________________________________________ > svn-src-all@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/svn-src-all > To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" > -- Sincerely, D.Marck [DM5020, MCK-RIPE, DM3-RIPN] [ FreeBSD committer: marck@FreeBSD.org ] ------------------------------------------------------------------------ *** Dmitry Morozovsky --- D.Marck --- Wild Woozle --- marck@rinet.ru *** ------------------------------------------------------------------------
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.1308221641010.10197>