From owner-freebsd-scsi@FreeBSD.ORG Thu Jul 31 09:14:25 2014 Return-Path: Delivered-To: freebsd-scsi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id BC54D8CE; Thu, 31 Jul 2014 09:14:25 +0000 (UTC) Received: from uriah.heep.sax.de (uriah.heep.sax.de [IPv6:2a01:170:1047::9]) by mx1.freebsd.org (Postfix) with ESMTP id 5DF752F62; Thu, 31 Jul 2014 09:14:25 +0000 (UTC) Received: by uriah.heep.sax.de (Postfix, from userid 107) id 045902CD4; Thu, 31 Jul 2014 11:14:22 +0200 (CEST) Date: Thu, 31 Jul 2014 11:14:22 +0200 From: Joerg Wunsch To: freebsd-scsi@freebsd.org, "Kenneth D. Merry" Subject: Re: Bacula fails on FreeBSD 10.x / "mt fsf" infinitely proceeds Message-ID: <20140731091422.GA37239@uriah.heep.sax.de> Mail-Followup-To: Joerg Wunsch , freebsd-scsi@freebsd.org, "Kenneth D. Merry" References: <20140730035230.GA81800@nargothrond.kdm.org> <20140730060330.GA3272@uriah.heep.sax.de> <20140730153229.GA86368@nargothrond.kdm.org> <20140730191915.9B944267B@uriah.heep.sax.de> <20140730203315.0EED1295B@uriah.heep.sax.de> <20140730204200.4645729B8@uriah.heep.sax.de> <53D95F61.4080701@FreeBSD.org> <20140730215113.GA3564@uriah.heep.sax.de> <20140731035756.GA91452@nargothrond.kdm.org> <20140731060936.GB4095@uriah.heep.sax.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="RnlQjJ0d97Da+TV1" Content-Disposition: inline In-Reply-To: <20140731060936.GB4095@uriah.heep.sax.de> X-Phone: +49-351-2012 669 X-PGP-Fingerprint: DC 47 E6 E4 FF A6 E9 8F 93 21 E0 7D F9 12 D6 4E X-GPG-Fingerprint: 5E84 F980 C3CA FD4B B584 1070 F48C A81B 69A8 5873 User-Agent: Mutt/1.5.23 (2014-03-12) X-BeenThere: freebsd-scsi@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: SCSI subsystem List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Jul 2014 09:14:25 -0000 --RnlQjJ0d97Da+TV1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline As Joerg Wunsch wrote: > > I think it would be good to keep the ability to report to the application > > or the user what happened with control and data commands, especially since > > you may wind up having errors on both types of commands in some cases. > > (e.g. end of media notification on a write, and then perhaps an issue in > > writing the filemarks.) > > OK, then it needs some more work. Next attempt: in saerror(), rather than basing the decision whether this status report belongs to a read/write or control operation on some CCB flag that needs to be set explicitly, base it on the actual command that led to the status report (SA_READ or SA_WRITE vs. anything else). A similar decision is already done in saerror(). I think this keeps the spirit of the existing implementation as much as possible. So far, only compile-tested. -- cheers, Joerg .-.-. --... ...-- -.. . DL8DTL http://www.sax.de/~joerg/ Never trust an operating system you don't have sources for. ;-) --RnlQjJ0d97Da+TV1 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="scsi_sa.c.diff" Index: scsi_sa.c =================================================================== --- scsi_sa.c (revision 269317) +++ scsi_sa.c (working copy) @@ -113,18 +113,10 @@ #define ccb_pflags ppriv_field0 #define ccb_bp ppriv_ptr1 -#define SA_CCB_BUFFER_IO 0x0 -#define SA_CCB_TYPEMASK 0x1 -#define SA_POSITION_UPDATED 0x2 +/* bits in ccb_pflags */ +#define SA_POSITION_UPDATED 0x1 -#define Set_CCB_Type(x, type) \ - x->ccb_h.ccb_pflags &= ~SA_CCB_TYPEMASK; \ - x->ccb_h.ccb_pflags |= type -#define CCB_Type(x) (x->ccb_h.ccb_pflags & SA_CCB_TYPEMASK) - - - typedef enum { SA_FLAG_OPEN = 0x0001, SA_FLAG_FIXED = 0x0002, @@ -1835,7 +1827,6 @@ bp->bio_data, bp->bio_bcount, SSD_FULL_SIZE, IO_TIMEOUT); start_ccb->ccb_h.ccb_pflags &= ~SA_POSITION_UPDATED; - Set_CCB_Type(start_ccb, SA_CCB_BUFFER_IO); start_ccb->ccb_h.ccb_bp = bp; bp = bioq_first(&softc->bio_queue); xpt_action(start_ccb); @@ -1860,92 +1851,86 @@ { struct sa_softc *softc; struct ccb_scsiio *csio; + struct bio *bp; + int error; softc = (struct sa_softc *)periph->softc; csio = &done_ccb->csio; - switch (CCB_Type(csio)) { - case SA_CCB_BUFFER_IO: - { - struct bio *bp; - int error; - softc->dsreg = MTIO_DSREG_REST; - bp = (struct bio *)done_ccb->ccb_h.ccb_bp; - error = 0; - if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { - if ((error = saerror(done_ccb, 0, 0)) == ERESTART) { - /* - * A retry was scheduled, so just return. - */ - return; - } + softc->dsreg = MTIO_DSREG_REST; + bp = (struct bio *)done_ccb->ccb_h.ccb_bp; + error = 0; + if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { + if ((error = saerror(done_ccb, 0, 0)) == ERESTART) { + /* + * A retry was scheduled, so just return. + */ + return; } + } - if (error == EIO) { + if (error == EIO) { - /* - * Catastrophic error. Mark the tape as frozen - * (we no longer know tape position). - * - * Return all queued I/O with EIO, and unfreeze - * our queue so that future transactions that - * attempt to fix this problem can get to the - * device. - * - */ + /* + * Catastrophic error. Mark the tape as frozen + * (we no longer know tape position). + * + * Return all queued I/O with EIO, and unfreeze + * our queue so that future transactions that + * attempt to fix this problem can get to the + * device. + * + */ - softc->flags |= SA_FLAG_TAPE_FROZEN; - bioq_flush(&softc->bio_queue, NULL, EIO); + softc->flags |= SA_FLAG_TAPE_FROZEN; + bioq_flush(&softc->bio_queue, NULL, EIO); + } + if (error != 0) { + bp->bio_resid = bp->bio_bcount; + bp->bio_error = error; + bp->bio_flags |= BIO_ERROR; + /* + * In the error case, position is updated in saerror. + */ + } else { + bp->bio_resid = csio->resid; + bp->bio_error = 0; + if (csio->resid != 0) { + bp->bio_flags |= BIO_ERROR; } - if (error != 0) { - bp->bio_resid = bp->bio_bcount; - bp->bio_error = error; - bp->bio_flags |= BIO_ERROR; - /* - * In the error case, position is updated in saerror. - */ - } else { - bp->bio_resid = csio->resid; - bp->bio_error = 0; - if (csio->resid != 0) { - bp->bio_flags |= BIO_ERROR; - } - if (bp->bio_cmd == BIO_WRITE) { - softc->flags |= SA_FLAG_TAPE_WRITTEN; - softc->filemarks = 0; - } - if (!(csio->ccb_h.ccb_pflags & SA_POSITION_UPDATED) && - (softc->blkno != (daddr_t) -1)) { - if ((softc->flags & SA_FLAG_FIXED) != 0) { - u_int32_t l; - if (softc->blk_shift != 0) { - l = bp->bio_bcount >> - softc->blk_shift; - } else { - l = bp->bio_bcount / - softc->media_blksize; - } - softc->blkno += (daddr_t) l; + if (bp->bio_cmd == BIO_WRITE) { + softc->flags |= SA_FLAG_TAPE_WRITTEN; + softc->filemarks = 0; + } + if (!(csio->ccb_h.ccb_pflags & SA_POSITION_UPDATED) && + (softc->blkno != (daddr_t) -1)) { + if ((softc->flags & SA_FLAG_FIXED) != 0) { + u_int32_t l; + if (softc->blk_shift != 0) { + l = bp->bio_bcount >> + softc->blk_shift; } else { - softc->blkno++; + l = bp->bio_bcount / + softc->media_blksize; } + softc->blkno += (daddr_t) l; + } else { + softc->blkno++; } } - /* - * If we had an error (immediate or pending), - * release the device queue now. - */ - if (error || (softc->flags & SA_FLAG_ERR_PENDING)) - cam_release_devq(done_ccb->ccb_h.path, 0, 0, 0, 0); - if (error || bp->bio_resid) { - CAM_DEBUG(periph->path, CAM_DEBUG_INFO, - ("error %d resid %ld count %ld\n", error, - bp->bio_resid, bp->bio_bcount)); - } - biofinish(bp, softc->device_stats, 0); - break; } + /* + * If we had an error (immediate or pending), + * release the device queue now. + */ + if (error || (softc->flags & SA_FLAG_ERR_PENDING)) + cam_release_devq(done_ccb->ccb_h.path, 0, 0, 0, 0); + if (error || bp->bio_resid) { + CAM_DEBUG(periph->path, CAM_DEBUG_INFO, + ("error %d resid %ld count %ld\n", error, + bp->bio_resid, bp->bio_bcount)); } + biofinish(bp, softc->device_stats, 0); xpt_release_ccb(done_ccb); } @@ -2498,7 +2483,8 @@ info /= softc->media_blksize; } } - if (CCB_Type(csio) == SA_CCB_BUFFER_IO) { + if (csio->cdb_io.cdb_bytes[0] == SA_READ || + csio->cdb_io.cdb_bytes[0] == SA_WRITE) { bcopy((caddr_t) sense, (caddr_t) &softc->last_io_sense, sizeof (struct scsi_sense_data)); bcopy(csio->cdb_io.cdb_bytes, softc->last_io_cdb, --RnlQjJ0d97Da+TV1--