On Sat, Aug 25, 2007 at 10:24:01AM +0200, Frank Kardel wrote:
> Hi Andrew, * !
> 
> I am experiencing quite a few strange error with my DAT tape since about
> this checkin. A kernel from the beginning of July works flawlessly with the
> drive and the tape.
> 
> Symptoms:
>     I/O errors claimed where no real I/O error where. Possibly
>    triggered by length indicator mismatches reported by the drive. These
>   are ok for variable block size formatted media. Those indications where
>   handled properly at the beginning of July and years before.
> 
> The dmesg output shows a READ that is terminated with an I/O error,
> but data was transferred (with the size mismatch indication).
> The requested block was just too big - a smaller block was read.
> The second attempt with a block too small was terminated with a panic 
> instead
> of an I/O error.
> 
> Could somebody check what broke st.c between beginning of July and now ?
> Currently st.c is unusable for variable block size tape I/O.
> 
> dmesg atteched.
> 
> Regards,
>  Frank
> 
> Andrew Doran wrote:
> >Module Name:	src
> >Committed By:	ad
> >Date:		Sun Jul 29 13:31:18 UTC 2007
> >
> >Modified Files:
> >	src/sys/arch/amiga/dev: fd.c
> >	src/sys/arch/dreamcast/dev/maple: mmemcard.c
> >	src/sys/dev/ieee1394: firewirereg.h fwdev.c fwmem.c
> >	src/sys/fs/cd9660: cd9660_vnops.c
> >	src/sys/fs/efs: efs_vnops.c
> >	src/sys/fs/ntfs: ntfs_vnops.c
> >	src/sys/fs/puffs: puffs_subr.c puffs_vnops.c
> >	src/sys/fs/smbfs: smbfs_io.c
> >	src/sys/fs/sysvbfs: sysvbfs_vnops.c
> >	src/sys/fs/udf: udf_subr.c
> >	src/sys/miscfs/deadfs: dead_vnops.c
> >	src/sys/miscfs/specfs: spec_vnops.c
> >	src/sys/nfs: nfs_bio.c
> >	src/sys/ufs/ffs: ffs_softdep.c
> >	src/sys/ufs/lfs: lfs_bio.c lfs_segment.c lfs_vfsops.c lfs_vnops.c
> >	src/sys/ufs/mfs: mfs_vnops.c
> >	src/sys/ufs/ufs: ufs_vnops.c
> >	src/sys/uvm: uvm_pager.c uvm_swap.c
> >
> >Log Message:
> >It's not a good idea for device drivers to modify b_flags, as they don't
> >need to understand the locking around that field. Instead of setting
> >B_ERROR, set b_error instead. b_error is 'owned' by whoever completes
> >the I/O request.
[snip]

Same here, reading a tape file yields EIO on end-of-file.  Looks like the
change to physio_done() removed the EOM handling.  Before we got

    mbp->b_error = 0;
    mbp->b_flags |= B_ERROR;

now we get

    mbp->b_error = EIO;

@@ -176,28 +176,16 @@ physio_done(struct work *wk, void *dummy
 
 		if (mbp->b_endoffset == -1 || endoffset < mbp->b_endoffset) {
-			int error;
-
-			if ((bp->b_flags & B_ERROR) != 0) {
-				if (bp->b_error == 0) {
-					error = EIO; /* XXX */
-				} else {
-					error = bp->b_error;
-				}
-			} else {
-				error = 0; /* EOM */
-			}
-
 			DPRINTF(("%s: mbp=%p, error %d -> %d, endoff %" PRIu64
 			    " -> %" PRIu64 "\n",
 			    __func__, mbp,
-			    mbp->b_error, error,
+			    mbp->b_error, bp->b_error,
 			    mbp->b_endoffset, endoffset));
 
 			mbp->b_endoffset = endoffset;
-			mbp->b_error = error;
+			mbp->b_error = bp->b_error;
 		}
-		mbp->b_flags |= B_ERROR;
+		mbp->b_error = EIO;
 	} else {
-		KASSERT((bp->b_flags & B_ERROR) == 0);
+		KASSERT(bp->b_error == 0);
 	}
 

-- 
Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig (Germany)