Subject: [I Think] sddone() Doesn't Count Partial xfers Correctly
To: None <netbsd-bugs@netbsd.org>
From: Chris Jepeway <jepeway@blasted-heath.com>
List: netbsd-bugs
Date: 03/28/2002 14:20:37
scsipi_complete() calls sddone() through the (*psw_done)() completion
hook in a scsipi_xfer struct. sddone() uses xs->bp to, among other
things, count up the # of bytes xferred to/from the disk by checking
the b_resid field of the buffer, as in
disk_unbusy(&sd->sc_dk, xs->bp->b_bcount - xs->bp->b_resid);
However, scspi_complete() calls through (*psw_done)() b/4 setting
xs->bp->b_resid to anything. I'm betting, then, that sddone() will
mis-count partial xfers.
This patch illustrates/fixes the problem. It's from a local repository
forked from NetBSD-current at revision 1.61 of scsipi_base.c, so
ignore its versions and line numbers:
Index: scsipi_base.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/scsipi_base.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- scsipi_base.c 2002/02/27 23:22:54 1.3
+++ scsipi_base.c 2002/03/07 17:37:59 1.4
@@ -1532,9 +1532,10 @@
if (xs->error != XS_NOERROR)
scsipi_periph_thaw(periph, 1);
-
- if (periph->periph_switch->psw_done)
- periph->periph_switch->psw_done(xs);
+ /*
+ * Set buffer fields in case the periph
+ * switch done func uses them
+ */
if ((bp = xs->bp) != NULL) {
if (error) {
bp->b_error = error;
@@ -1543,9 +1544,13 @@
} else {
bp->b_error = 0;
bp->b_resid = xs->resid;
- }
- biodone(bp);
+ }
}
+
+ if (periph->periph_switch->psw_done)
+ periph->periph_switch->psw_done(xs);
+ if (bp)
+ biodone(bp);
if (xs->xs_control & XS_CTL_ASYNC)
scsipi_put_xs(xs);
Is a better fix to make sddone() use the values in the xs proper,
instead of using those in xs->bp? Such an approach was suggested
back in late 96 in kern/3031, but it's clear it wasn't taken.
But, nevermind the fix, should I file a PR on this?
Chris <jepeway@blasted-heath.com>.