From owner-svn-src-head@FreeBSD.ORG Thu Aug 22 12:47:25 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 5489BD9D; Thu, 22 Aug 2013 12:47:25 +0000 (UTC) (envelope-from marck@rinet.ru) Received: from woozle.rinet.ru (woozle.rinet.ru [195.54.192.68]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id CBE6B2A73; Thu, 22 Aug 2013 12:47:24 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by woozle.rinet.ru (8.14.5/8.14.5) with ESMTP id r7MCgfrY010549; Thu, 22 Aug 2013 16:42:41 +0400 (MSK) (envelope-from marck@rinet.ru) Date: Thu, 22 Aug 2013 16:42:41 +0400 (MSK) From: Dmitry Morozovsky To: "Kenneth D. Merry" Subject: Re: svn commit: r254615 - head/sys/dev/mps In-Reply-To: <201308212130.r7LLUvO5008991@svn.freebsd.org> Message-ID: References: <201308212130.r7LLUvO5008991@svn.freebsd.org> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) X-NCC-RegID: ru.rinet X-OpenPGP-Key-ID: 6B691B03 MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (woozle.rinet.ru [0.0.0.0]); Thu, 22 Aug 2013 16:42:41 +0400 (MSK) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Aug 2013 12:47:25 -0000 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 *** ------------------------------------------------------------------------