svn commit: r211434 - head/sys/cam/scsi

[Scott Long]

This is violates the policy that CAM has effectively had for a long time that separates protocol error handling in the periph from transport error recovery in the SIM.  I think it's better to encourage SIMs to register an AC_LOST_DEVICE event and handle command aborts themselves.  Most drivers have a busy queue and know what outstanding CCBs belong to what devices/targets.  And in the age of SAS, the SIMs are going to know about dead devices long before the periph will, and should already be in the process of aborting the outstanding commands and returning the CCBs.  SIMs that don't probably don't even properly timeout outstanding commands, so they'll have much deeper problems than this.

I'm in the middle of working on this for the mps driver.  Your change may or may not get in the way of the design I already have, where the SIM autonomously dequeues and completes all outstanding CCBs to dead devices.  I'll hopefully have an answer in the coming weeks and let you know.  Please don't MFC before then.

Scott



On Aug 17, 2010, at 11:11 AM, Matt Jacob wrote:

> Author: mjacob
> Date: Tue Aug 17 17:11:15 2010
> New Revision: 211434
> URL: http://svn.freebsd.org/changeset/base/211434
> 
> Log:
>  Now is as good a time as any to find out if we induce breakage
>  by issueing aborts for any pending commands when we're decommssioning
>  a disk.
> 
>  MFC after:	3 months
> 
> Modified:
>  head/sys/cam/scsi/scsi_da.c
> 
> Modified: head/sys/cam/scsi/scsi_da.c
> ==============================================================================
> --- head/sys/cam/scsi/scsi_da.c	Tue Aug 17 16:41:16 2010	(r211433)
> +++ head/sys/cam/scsi/scsi_da.c	Tue Aug 17 17:11:15 2010	(r211434)
> @@ -958,6 +958,8 @@ dainit(void)
> static void
> daoninvalidate(struct cam_periph *periph)
> {
> +	struct ccb_abort cab;
> +	struct ccb_hdr *ccb_h, *ccb_h_t;
> 	struct da_softc *softc;
> 
> 	softc = (struct da_softc *)periph->softc;
> @@ -967,15 +969,29 @@ daoninvalidate(struct cam_periph *periph
> 	 */
> 	xpt_register_async(0, daasync, periph, periph->path);
> 
> +	/*
> +	 * Invalidate the pack label
> +	 */
> 	softc->flags |= DA_FLAG_PACK_INVALID;
> 
> 	/*
> 	 * Return all queued I/O with ENXIO.
> -	 * XXX Handle any transactions queued to the card
> -	 *     with XPT_ABORT_CCB.
> 	 */
> 	bioq_flush(&softc->bio_queue, NULL, ENXIO);
> 
> +	/*
> +	 * Issue aborts for any pending commands.
> +	 */
> +	xpt_setup_ccb(&cab.ccb_h, periph->path, CAM_PRIORITY_NORMAL+1);
> +	cab.ccb_h.func_code = XPT_ABORT;
> +	LIST_FOREACH_SAFE(ccb_h, &softc->pending_ccbs, periph_links.le, ccb_h_t) {
> +		cab.abort_ccb = (union ccb *)ccb_h;
> +		xpt_action((union ccb *)&cab);
> +	}
> +
> +	/*
> +	 * This disk is *history*....
> +	 */
> 	disk_gone(softc->disk);
> 	xpt_print(periph->path, "lost device\n");
> }