On Mon, Jul 04, 2011 at 05:11:35PM -0700, Matthew Dempsky wrote:
> Once this goes in, I'll start updating the xxstrategy() methods to
> remove these redundant checks.
And here's my promised followup diff.
The convention now in xxstrategy() functions is that the 'bad' label
sets B_ERROR and sets b_resid = b_bcount, and then the 'done' label
just calls biodone(). There were a few inconsistencies where error
paths were not setting b_resid = b_bcount.
I also skipped raidframe and vax's uncompilable hp(4) driver that Miod
promises he'll delete REAL SOON NOW... !!! :)
ok?
Index: ./arch/hp300/dev/hd.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/arch/hp300/dev/hd.c,v
retrieving revision 1.68
diff -u -p -r1.68 hd.c
--- ./arch/hp300/dev/hd.c 19 Jun 2011 21:20:04 -0000 1.68
+++ ./arch/hp300/dev/hd.c 5 Jul 2011 03:28:50 -0000
@@ -650,7 +650,6 @@ hdstrategy(bp)
int unit = DISKUNIT(bp->b_dev);
struct hd_softc *rs;
struct buf *dp;
- struct disklabel *lp;
int s;
rs = hdlookup(unit);
@@ -666,27 +665,8 @@ hdstrategy(bp)
(bp->b_flags & B_READ) ? 'R' : 'W');
#endif
- lp = rs->sc_dkdev.dk_label;
-
- /*
- * If it's a null transfer, return immediately
- */
- if (bp->b_bcount == 0)
- goto done;
-
- /*
- * The transfer must be a whole number of blocks.
- */
- if ((bp->b_bcount % lp->d_secsize) != 0) {
- bp->b_error = EINVAL;
- goto bad;
- }
-
- /*
- * Do bounds checking, adjust transfer. if error, process;
- * If end of partition, just return.
- */
- if (bounds_check_with_label(bp, lp) <= 0)
+ /* Validate the request. */
+ if (bounds_check_with_label(bp, rs->sc_dkdev.dk_label) <= 0)
goto done;
s = splbio();
@@ -700,13 +680,11 @@ hdstrategy(bp)
device_unref(&rs->sc_dev);
return;
-bad:
+
+ bad:
bp->b_flags |= B_ERROR;
-done:
- /*
- * Correctly set the buf to indicate a completed xfer
- */
bp->b_resid = bp->b_bcount;
+ done:
s = splbio();
biodone(bp);
splx(s);
Index: ./arch/octeon/dev/octcf.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/arch/octeon/dev/octcf.c,v
retrieving revision 1.6
diff -u -p -r1.6 octcf.c
--- ./arch/octeon/dev/octcf.c 19 Jun 2011 04:55:34 -0000 1.6
+++ ./arch/octeon/dev/octcf.c 5 Jul 2011 03:38:18 -0000
@@ -302,27 +302,22 @@ octcfstrategy(struct buf *bp)
}
OCTCFDEBUG_PRINT(("octcfstrategy (%s)\n", wd->sc_dev.dv_xname),
DEBUG_XFERS);
- /* Valid request? */
- if (bp->b_blkno < 0 ||
- (bp->b_bcount % wd->sc_dk.dk_label->d_secsize) != 0 ||
- (bp->b_bcount / wd->sc_dk.dk_label->d_secsize) >= (1 << NBBY)) {
- bp->b_error = EINVAL;
- goto bad;
- }
/* If device invalidated (e.g. media change, door open), error. */
if ((wd->sc_flags & OCTCFF_LOADED) == 0) {
bp->b_error = EIO;
goto bad;
}
- /* If it's a null transfer, return immediately. */
- if (bp->b_bcount == 0)
- goto done;
- /*
- * Do bounds checking, adjust transfer. if error, process.
- * If end of partition, just return.
- */
+
+ /* Validate the request. */
if (bounds_check_with_label(bp, wd->sc_dk.dk_label) <= 0)
goto done;
+
+ /* Check that the number of sectors can fit in a byte. */
+ if ((bp->b_bcount / wd->sc_dk.dk_label->d_secsize) >= (1 << NBBY)) {
+ bp->b_error = EINVAL;
+ goto bad;
+ }
+
/* Queue transfer on drive, activate drive and controller if idle. */
s = splbio();
disksort(&wd->sc_q, bp);
@@ -330,11 +325,11 @@ octcfstrategy(struct buf *bp)
splx(s);
device_unref(&wd->sc_dev);
return;
-bad:
+
+ bad:
bp->b_flags |= B_ERROR;
-done:
- /* Toss transfer; we're done early. */
bp->b_resid = bp->b_bcount;
+ done:
s = splbio();
biodone(bp);
splx(s);
Index: ./arch/sparc/dev/presto.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/arch/sparc/dev/presto.c,v
retrieving revision 1.21
diff -u -p -r1.21 presto.c
--- ./arch/sparc/dev/presto.c 3 Jun 2011 21:14:11 -0000 1.21
+++ ./arch/sparc/dev/presto.c 5 Jul 2011 03:32:47 -0000
@@ -280,15 +280,14 @@ prestostrategy(struct buf *bp)
sc = (struct presto_softc *)device_lookup(&presto_cd, unit);
/* Sort rogue requests out */
- if (sc == NULL || bp->b_blkno < 0 ||
- (bp->b_bcount % sc->sc_dk.dk_label->d_secsize) != 0) {
+ if (sc == NULL) {
bp->b_error = EINVAL;
goto bad;
}
- /* Do not write on "no trespassing" areas... */
+ /* Validate the request. */
if (bounds_check_with_label(bp, sc->sc_dk.dk_label) <= 0)
- goto bad;
+ goto done;
/* Bound the request size, then move data between buf and nvram */
bp->b_resid = bp->b_bcount;
@@ -303,11 +302,10 @@ prestostrategy(struct buf *bp)
bp->b_resid -= count;
goto done;
-bad:
+ bad:
bp->b_flags |= B_ERROR;
bp->b_resid = bp->b_bcount;
-
-done:
+ done:
s = splbio();
biodone(bp);
splx(s);
Index: ./arch/sparc/dev/xd.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/arch/sparc/dev/xd.c,v
retrieving revision 1.55
diff -u -p -r1.55 xd.c
--- ./arch/sparc/dev/xd.c 5 Jun 2011 18:40:33 -0000 1.55
+++ ./arch/sparc/dev/xd.c 5 Jul 2011 03:24:14 -0000
@@ -1011,9 +1011,7 @@ xdstrategy(bp)
/* check for live device */
- if (unit >= xd_cd.cd_ndevs || (xd = xd_cd.cd_devs[unit]) == 0 ||
- bp->b_blkno < 0 ||
- (bp->b_bcount % xd->sc_dk.dk_label->d_secsize) != 0) {
+ if (unit >= xd_cd.cd_ndevs || (xd = xd_cd.cd_devs[unit]) == 0) {
bp->b_error = EINVAL;
goto bad;
}
@@ -1036,16 +1034,8 @@ xdstrategy(bp)
bp->b_error = EIO;
goto bad;
}
- /* short circuit zero length request */
-
- if (bp->b_bcount == 0)
- goto done;
-
- /* check bounds with label (disksubr.c). Determine the size of the
- * transfer, and make sure it is within the boundaries of the
- * partition. Adjust transfer if needed, and signal errors or early
- * completion. */
+ /* Validate the request. */
if (bounds_check_with_label(bp, xd->sc_dk.dk_label) <= 0)
goto done;
@@ -1090,11 +1080,11 @@ xdstrategy(bp)
splx(s);
return;
-bad: /* tells upper layers we have an error */
+ bad: /* tells upper layers we have an error */
bp->b_flags |= B_ERROR;
-done: /* tells upper layers we are done with this
- * buf */
bp->b_resid = bp->b_bcount;
+ done: /* tells upper layers we are done with this
+ * buf */
s = splbio();
biodone(bp);
splx(s);
Index: ./arch/sparc/dev/xy.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/arch/sparc/dev/xy.c,v
retrieving revision 1.52
diff -u -p -r1.52 xy.c
--- ./arch/sparc/dev/xy.c 5 Jun 2011 18:40:33 -0000 1.52
+++ ./arch/sparc/dev/xy.c 5 Jul 2011 03:23:29 -0000
@@ -974,9 +974,7 @@ xystrategy(bp)
/* check for live device */
- if (unit >= xy_cd.cd_ndevs || (xy = xy_cd.cd_devs[unit]) == 0 ||
- bp->b_blkno < 0 ||
- (bp->b_bcount % xy->sc_dk.dk_label->d_secsize) != 0) {
+ if (unit >= xy_cd.cd_ndevs || (xy = xy_cd.cd_devs[unit]) == 0) {
bp->b_error = EINVAL;
goto bad;
}
@@ -999,16 +997,8 @@ xystrategy(bp)
bp->b_error = EIO;
goto bad;
}
- /* short circuit zero length request */
-
- if (bp->b_bcount == 0)
- goto done;
-
- /* check bounds with label (disksubr.c). Determine the size of the
- * transfer, and make sure it is within the boundaries of the
- * partition. Adjust transfer if needed, and signal errors or early
- * completion. */
+ /* Validate the request. */
if (bounds_check_with_label(bp, xy->sc_dk.dk_label) <= 0)
goto done;
@@ -1029,11 +1019,11 @@ xystrategy(bp)
splx(s);
return;
-bad: /* tells upper layers we have an error */
+ bad: /* tells upper layers we have an error */
bp->b_flags |= B_ERROR;
-done: /* tells upper layers we are done with this
- * buf */
bp->b_resid = bp->b_bcount;
+ done: /* tells upper layers we are done with this
+ * buf */
s = splbio();
biodone(bp);
splx(s);
Index: ./arch/vax/mscp/mscp_disk.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/arch/vax/mscp/mscp_disk.c,v
retrieving revision 1.35
diff -u -p -r1.35 mscp_disk.c
--- ./arch/vax/mscp/mscp_disk.c 5 Jun 2011 18:40:33 -0000 1.35
+++ ./arch/vax/mscp/mscp_disk.c 5 Jul 2011 03:21:29 -0000
@@ -286,8 +286,7 @@ rastrategy(bp)
unit = DISKUNIT(bp->b_dev);
if (unit >= ra_cd.cd_ndevs || (ra = ra_cd.cd_devs[unit]) == NULL) {
bp->b_error = ENXIO;
- bp->b_flags |= B_ERROR;
- goto done;
+ goto bad;
}
/*
* If drive is open `raw' or reading label, let it at it.
@@ -303,9 +302,8 @@ rastrategy(bp)
/* If disk is not online, try to put it online */
if (ra->ra_state == DK_CLOSED)
if (ra_putonline(ra) == MSCP_FAILED) {
- bp->b_flags |= B_ERROR;
bp->b_error = EIO;
- goto done;
+ goto bad;
}
/*
@@ -322,7 +320,10 @@ rastrategy(bp)
mscp_strategy(bp, ra->ra_dev.dv_parent);
return;
-done:
+ bad:
+ bp->b_flags |= B_ERROR;
+ bp->b_resid = bp->b_bcount;
+ done:
s = splbio();
biodone(bp);
splx(s);
Index: ./arch/vax/vsa/hdc9224.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/arch/vax/vsa/hdc9224.c,v
retrieving revision 1.36
diff -u -p -r1.36 hdc9224.c
--- ./arch/vax/vsa/hdc9224.c 5 Jun 2011 18:40:33 -0000 1.36
+++ ./arch/vax/vsa/hdc9224.c 5 Jul 2011 03:34:17 -0000
@@ -440,22 +440,19 @@ hdstrategy(struct buf *bp)
{
struct hdsoftc *hd;
struct hdcsoftc *sc;
- struct disklabel *lp;
int unit, s;
unit = DISKUNIT(bp->b_dev);
if (unit > hd_cd.cd_ndevs || (hd = hd_cd.cd_devs[unit]) == NULL) {
bp->b_error = ENXIO;
bp->b_flags |= B_ERROR;
+ bp->b_resid = bp->b_bcount;
goto done;
}
sc = (void *)hd->sc_dev.dv_parent;
- lp = hd->sc_disk.dk_label;
- if ((bounds_check_with_label(bp, hd->sc_disk.dk_label)) <= 0)
- goto done;
-
- if (bp->b_bcount == 0)
+ /* Validate the request. */
+ if (bounds_check_with_label(bp, hd->sc_disk.dk_label) <= 0)
goto done;
s = splbio();
Index: ./dev/ccd.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/dev/ccd.c,v
retrieving revision 1.96
diff -u -p -r1.96 ccd.c
--- ./dev/ccd.c 30 Jun 2011 16:28:05 -0000 1.96
+++ ./dev/ccd.c 5 Jul 2011 03:19:54 -0000
@@ -614,28 +614,18 @@ ccdstrategy(struct buf *bp)
int unit = DISKUNIT(bp->b_dev);
struct ccd_softc *cs = &ccd_softc[unit];
int s;
- struct disklabel *lp;
CCD_DPRINTF(CCDB_FOLLOW, ("ccdstrategy(%p): unit %d\n", bp, unit));
if ((cs->sc_flags & CCDF_INITED) == 0) {
bp->b_error = ENXIO;
- bp->b_resid = bp->b_bcount;
bp->b_flags |= B_ERROR;
+ bp->b_resid = bp->b_bcount;
goto done;
}
- /* If it's a nil transfer, wake up the top half now. */
- if (bp->b_bcount == 0)
- goto done;
-
- lp = cs->sc_dkdev.dk_label;
-
- /*
- * Do bounds checking and adjust transfer. If there's an
- * error, the bounds check will flag that for us.
- */
- if (bounds_check_with_label(bp, lp) <= 0)
+ /* Validate the request. */
+ if (bounds_check_with_label(bp, cs->sc_dkdev.dk_label) <= 0)
goto done;
bp->b_resid = bp->b_bcount;
@@ -647,7 +637,8 @@ ccdstrategy(struct buf *bp)
ccdstart(cs, bp);
splx(s);
return;
-done:
+
+ done:
s = splbio();
biodone(bp);
splx(s);
Index: ./dev/flash.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/dev/flash.c,v
retrieving revision 1.23
diff -u -p -r1.23 flash.c
--- ./dev/flash.c 19 Jun 2011 04:55:33 -0000 1.23
+++ ./dev/flash.c 5 Jul 2011 03:18:16 -0000
@@ -784,12 +784,6 @@ flashstrategy(struct buf *bp)
goto bad;
}
- /* Transfer only a multiple of the flash page size. */
- if ((bp->b_bcount % sc->sc_flashdev->pagesize) != 0) {
- bp->b_error = EINVAL;
- goto bad;
- }
-
/* If the device has been invalidated, error out. */
if ((sc->sc_flags & FDK_LOADED) == 0) {
bp->b_error = EIO;
@@ -800,11 +794,7 @@ flashstrategy(struct buf *bp)
if (flashsafe(bp->b_dev) && flashsafestrategy(sc, bp) <= 0)
goto done;
- /* Return immediately if it is a null transfer. */
- if (bp->b_bcount == 0)
- goto done;
-
- /* Do bounds checking on partitions. */
+ /* Validate the request. */
if (bounds_check_with_label(bp, sc->sc_dk.dk_label) <= 0)
goto done;
@@ -816,11 +806,10 @@ flashstrategy(struct buf *bp)
device_unref(&sc->sc_dev);
return;
-bad:
+ bad:
bp->b_flags |= B_ERROR;
-done:
- if ((bp->b_flags & B_ERROR) != 0)
- bp->b_resid = bp->b_bcount;
+ bp->b_resid = bp->b_bcount;
+ done:
s = splbio();
biodone(bp);
splx(s);
Index: ./dev/rd.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/dev/rd.c,v
retrieving revision 1.4
diff -u -p -r1.4 rd.c
--- ./dev/rd.c 30 Jun 2011 16:28:05 -0000 1.4
+++ ./dev/rd.c 5 Jul 2011 03:16:26 -0000
@@ -215,17 +215,7 @@ rdstrategy(struct buf *bp)
goto bad;
}
- /* If it's a null transfer, return immediately. */
- if (bp->b_bcount == 0)
- goto done;
-
- /* The transfer must be a whole number of sectors. */
- if ((bp->b_bcount % sc->sc_dk.dk_label->d_secsize) != 0) {
- bp->b_error = EINVAL;
- goto bad;
- }
-
- /* Check that the request is within the partition boundaries. */
+ /* Validate the request. */
if (bounds_check_with_label(bp, sc->sc_dk.dk_label) <= 0)
goto done;
@@ -250,11 +240,13 @@ rdstrategy(struct buf *bp)
bad:
bp->b_flags |= B_ERROR;
+ bp->b_resid = bp->b_bcount;
done:
s = splbio();
biodone(bp);
splx(s);
- device_unref(&sc->sc_dev);
+ if (sc != NULL)
+ device_unref(&sc->sc_dev);
}
int
Index: ./dev/ata/wd.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/dev/ata/wd.c,v
retrieving revision 1.107
diff -u -p -r1.107 wd.c
--- ./dev/ata/wd.c 30 Jun 2011 16:28:05 -0000 1.107
+++ ./dev/ata/wd.c 5 Jul 2011 03:38:08 -0000
@@ -407,30 +407,22 @@ wdstrategy(struct buf *bp)
WDCDEBUG_PRINT(("wdstrategy (%s)\n", wd->sc_dev.dv_xname),
DEBUG_XFERS);
- /* Valid request? */
- if (bp->b_blkno < 0 ||
- (bp->b_bcount % wd->sc_dk.dk_label->d_secsize) != 0 ||
- (bp->b_bcount / wd->sc_dk.dk_label->d_secsize) >= (1 << NBBY)) {
- bp->b_error = EINVAL;
- goto bad;
- }
-
/* If device invalidated (e.g. media change, door open), error. */
if ((wd->sc_flags & WDF_LOADED) == 0) {
bp->b_error = EIO;
goto bad;
}
- /* If it's a null transfer, return immediately. */
- if (bp->b_bcount == 0)
- goto done;
-
- /*
- * Do bounds checking, adjust transfer. if error, process.
- * If end of partition, just return.
- */
+ /* Validate the request. */
if (bounds_check_with_label(bp, wd->sc_dk.dk_label) <= 0)
goto done;
+
+ /* Check that the number of sectors can fit in a byte. */
+ if ((bp->b_bcount / wd->sc_dk.dk_label->d_secsize) >= (1 << NBBY)) {
+ bp->b_error = EINVAL;
+ goto bad;
+ }
+
/* Queue transfer on drive, activate drive and controller if idle. */
bufq_queue(&wd->sc_bufq, bp);
s = splbio();
@@ -438,11 +430,11 @@ wdstrategy(struct buf *bp)
splx(s);
device_unref(&wd->sc_dev);
return;
-bad:
+
+ bad:
bp->b_flags |= B_ERROR;
-done:
- /* Toss transfer; we're done early. */
bp->b_resid = bp->b_bcount;
+ done:
s = splbio();
biodone(bp);
splx(s);
Index: ./scsi/cd.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/scsi/cd.c,v
retrieving revision 1.206
diff -u -p -r1.206 cd.c
--- ./scsi/cd.c 3 Jul 2011 15:47:18 -0000 1.206
+++ ./scsi/cd.c 5 Jul 2011 03:12:00 -0000
@@ -466,23 +466,8 @@ cdstrategy(struct buf *bp)
bp->b_error = EIO;
goto bad;
}
- /*
- * The transfer must be a whole number of blocks.
- */
- if ((bp->b_bcount % sc->sc_dk.dk_label->d_secsize) != 0) {
- bp->b_error = EINVAL;
- goto bad;
- }
- /*
- * If it's a null transfer, return immediately
- */
- if (bp->b_bcount == 0)
- goto done;
- /*
- * Do bounds checking, adjust transfer. if error, process.
- * If end of partition, just return.
- */
+ /* Validate the request. */
if (bounds_check_with_label(bp, sc->sc_dk.dk_label) <= 0)
goto done;
@@ -498,13 +483,10 @@ cdstrategy(struct buf *bp)
device_unref(&sc->sc_dev);
return;
-bad:
+ bad:
bp->b_flags |= B_ERROR;
-done:
- /*
- * Set the buf to indicate no xfer was done.
- */
bp->b_resid = bp->b_bcount;
+ done:
s = splbio();
biodone(bp);
splx(s);
Index: ./scsi/sd.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/scsi/sd.c,v
retrieving revision 1.232
diff -u -p -r1.232 sd.c
--- ./scsi/sd.c 3 Jul 2011 15:47:18 -0000 1.232
+++ ./scsi/sd.c 5 Jul 2011 03:40:51 -0000
@@ -517,23 +517,8 @@ sdstrategy(struct buf *bp)
bp->b_error = ENODEV;
goto bad;
}
- /*
- * If it's a null transfer, return immediately
- */
- if (bp->b_bcount == 0)
- goto done;
- /*
- * The transfer must be a whole number of sectors.
- */
- if ((bp->b_bcount % sc->sc_dk.dk_label->d_secsize) != 0) {
- bp->b_error = EINVAL;
- goto bad;
- }
- /*
- * Do bounds checking, adjust transfer. if error, process.
- * If end of partition, just return.
- */
+ /* Validate the request. */
if (bounds_check_with_label(bp, sc->sc_dk.dk_label) <= 0)
goto done;
@@ -549,13 +534,10 @@ sdstrategy(struct buf *bp)
device_unref(&sc->sc_dev);
return;
-bad:
+ bad:
bp->b_flags |= B_ERROR;
-done:
- /*
- * Correctly set the buf to indicate a completed xfer
- */
bp->b_resid = bp->b_bcount;
+ done:
s = splbio();
biodone(bp);
splx(s);
