svn commit: r254615 - head/sys/dev/mps

[Dmitry Morozovsky]

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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"
> 

-- 
Sincerely,
D.Marck                                     [DM5020, MCK-RIPE, DM3-RIPN]
[ FreeBSD committer:                                 marck at FreeBSD.org ]
------------------------------------------------------------------------
*** Dmitry Morozovsky --- D.Marck --- Wild Woozle --- marck at rinet.ru ***
------------------------------------------------------------------------