diff options
author | grog <grog@FreeBSD.org> | 2000-05-04 07:44:23 +0000 |
---|---|---|
committer | grog <grog@FreeBSD.org> | 2000-05-04 07:44:23 +0000 |
commit | bbbefd438c69a46d89e532fabc06e1a96287d3be (patch) | |
tree | ae6d6207e7de489e6fb5091929a46ed910be47f7 | |
parent | 701969bf4a6cb9e0b3b22be1e78ee8b4f6be3044 (diff) | |
download | FreeBSD-src-bbbefd438c69a46d89e532fabc06e1a96287d3be.zip FreeBSD-src-bbbefd438c69a46d89e532fabc06e1a96287d3be.tar.gz |
Don't hide bio structure behind macros like b_ioflags.
Get counting volume I/Os right.
launch_requests: Be macho, throw away the safety net and walk the
tightrope with no splbio().
Add some comments explaining the smoke and mirrors.
Remove some redundant braces.
sdio: Set the state of an accessed but down subdisk correctly. This
appears to duplicate an earlier commit that I hadn't seen.
-rw-r--r-- | sys/dev/vinum/vinumrequest.c | 94 |
1 files changed, 54 insertions, 40 deletions
diff --git a/sys/dev/vinum/vinumrequest.c b/sys/dev/vinum/vinumrequest.c index 62fe2ce..da77b69 100644 --- a/sys/dev/vinum/vinumrequest.c +++ b/sys/dev/vinum/vinumrequest.c @@ -1,5 +1,5 @@ /*- - * Copyright (c) 1997, 1998, 1999 + * Copyright) 1997, 1998, 1999 * Nan Yang Computer Services Limited. All rights reserved. * * Parts copyright (c) 1997, 1998 Cybernet Corporation, NetMAX project. @@ -117,9 +117,9 @@ logrq(enum rqinfo_type type, union rqinfou info, struct buf *ubp) #endif void -vinumstrategy(struct bio *bip) +vinumstrategy(struct bio *biop) { - struct buf *bp = (struct buf *) bip; + struct buf *bp = (struct buf *) biop; int volno; struct volume *vol = NULL; @@ -136,7 +136,7 @@ vinumstrategy(struct bio *bip) case VINUM_DRIVE_TYPE: default: bp->b_error = EIO; /* I/O error */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bufdone(bp); return; @@ -145,12 +145,12 @@ vinumstrategy(struct bio *bip) vol = &VOL[volno]; if (vol->state != volume_up) { /* can't access this volume */ bp->b_error = EIO; /* I/O error */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bufdone(bp); return; } if (vinum_bounds_check(bp, vol) <= 0) { /* don't like them bounds */ - bufdone(bp); /* have nothing to do with this */ + bufdone(bp); return; } /* FALLTHROUGH */ @@ -193,14 +193,14 @@ vinumstart(struct buf *bp, int reviveok) if ((bp->b_bcount % DEV_BSIZE) != 0) { /* bad length */ bp->b_error = EINVAL; /* invalid size */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bufdone(bp); return -1; } rq = (struct request *) Malloc(sizeof(struct request)); /* allocate a request struct */ if (rq == NULL) { /* can't do it */ bp->b_error = ENOMEM; /* can't get memory */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bufdone(bp); return -1; } @@ -237,7 +237,6 @@ vinumstart(struct buf *bp, int reviveok) * effects, however. */ if (vol != NULL) { - vol->reads++; plexno = vol->preferred_plex; /* get the plex to use */ if (plexno < 0) { /* round robin */ plexno = vol->last_plex_read; @@ -258,7 +257,7 @@ vinumstart(struct buf *bp, int reviveok) ||(bp->b_flags & B_DONE)) { /* XXX shouldn't get this without bad status */ if (status == REQUEST_DOWN) { /* not enough subdisks */ bp->b_error = EIO; /* I/O error */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; } bufdone(bp); freerq(rq); @@ -271,10 +270,9 @@ vinumstart(struct buf *bp, int reviveok) * a RAID-4 or RAID-5 plex, we must also update the parity stripe. */ { - if (vol != NULL) { - vol->writes++; + if (vol != NULL) status = build_write_request(rq); /* Not all the subdisks are up */ - } else { /* plex I/O */ + else { /* plex I/O */ daddr_t diskstart; diskstart = bp->b_blkno; /* start offset of transfer */ @@ -287,7 +285,7 @@ vinumstart(struct buf *bp, int reviveok) ||(bp->b_flags & B_DONE)) { /* XXX shouldn't get this without bad status */ if (status == REQUEST_DOWN) { /* not enough subdisks */ bp->b_error = EIO; /* I/O error */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; } if ((bp->b_flags & B_DONE) == 0) bufdone(bp); @@ -305,7 +303,6 @@ vinumstart(struct buf *bp, int reviveok) int launch_requests(struct request *rq, int reviveok) { - int s; struct rqgroup *rqg; int rqno; /* loop index */ struct rqelement *rqe; /* current element */ @@ -318,6 +315,7 @@ launch_requests(struct request *rq, int reviveok) * the request off plex->waitlist of the first * plex we find which is reviving */ + if ((rq->flags & XFR_REVIVECONFLICT) /* possible revive conflict */ &&(!reviveok)) { /* and we don't want to do it now, */ struct sd *sd; @@ -364,11 +362,23 @@ launch_requests(struct request *rq, int reviveok) #endif /* - * With the division of labour below (first count the requests, then - * issue them), it's possible that we don't need this splbio() - * protection. But I'll try that some other time. + * We used to have an splbio() here anyway, out + * of superstition. With the division of labour + * below (first count the requests, then issue + * them), it looks as if we don't need this + * splbio() protection. In fact, as dillon + * points out, there's a race condition + * incrementing and decrementing rq->active and + * rqg->active. This splbio() didn't help + * there, because the device strategy routine + * can sleep. Solve this by putting shorter + * duration locks on the code. + */ + /* + * This loop happens without any participation + * of the bottom half, so it requires no + * protection. */ - s = splbio(); for (rqg = rq->rqg; rqg != NULL; rqg = rqg->next) { /* through the whole request chain */ rqg->active = rqg->count; /* they're all active */ for (rqno = 0; rqno < rqg->count; rqno++) { @@ -380,7 +390,11 @@ launch_requests(struct request *rq, int reviveok) rq->active++; /* one more active request group */ } - /* Now fire off the requests */ + /* + * Now fire off the requests. In this loop the + * bottom half could be completing requests + * before we finish, so we need splbio() protection. + */ for (rqg = rq->rqg; rqg != NULL;) { /* through the whole request chain */ if (rqg->lockbase >= 0) /* this rqg needs a lock first */ rqg->lock = lockrange(rqg->lockbase, rqg->rq->bp, &PLEX[rqg->plexno]); @@ -403,7 +417,7 @@ launch_requests(struct request *rq, int reviveok) if (vinum_conf.active >= vinum_conf.maxactive) vinum_conf.maxactive = vinum_conf.active; -#if VINUMDEBUG +#ifdef VINUMDEBUG if (debug & DEBUG_ADDRESSES) log(LOG_DEBUG, " %s dev %d.%d, sd %d, offset 0x%x, devoffset 0x%x, length %ld\n", @@ -417,14 +431,11 @@ launch_requests(struct request *rq, int reviveok) if (debug & DEBUG_LASTREQS) logrq(loginfo_rqe, (union rqinfou) rqe, rq->bp); #endif - - /* fire off the request */ DEV_STRATEGY(&rqe->b, 0); } } } - splx(s); return 0; } @@ -481,7 +492,7 @@ bre(struct request *rq, if (*diskaddr < (sd->plexoffset + sd->sectors)) { /* the request starts in this subdisk */ rqg = allocrqg(rq, 1); /* space for the request */ if (rqg == NULL) { /* malloc failed */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bp->b_error = ENOMEM; bufdone(bp); return REQUEST_ENOMEM; @@ -520,7 +531,7 @@ bre(struct request *rq, *diskaddr += rqe->datalen; /* bump the address */ if (build_rq_buffer(rqe, plex)) { /* build the buffer */ deallocrqg(rqg); - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bp->b_error = ENOMEM; bufdone(bp); return REQUEST_ENOMEM; /* can't do it */ @@ -565,7 +576,7 @@ bre(struct request *rq, sd = &SD[plex->sdnos[sdno]]; /* the subdisk in question */ rqg = allocrqg(rq, 1); /* space for the request */ if (rqg == NULL) { /* malloc failed */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bp->b_error = ENOMEM; bufdone(bp); return REQUEST_ENOMEM; @@ -630,7 +641,7 @@ bre(struct request *rq, } if (build_rq_buffer(rqe, plex)) { /* build the buffer */ deallocrqg(rqg); - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bp->b_error = ENOMEM; bufdone(bp); return REQUEST_ENOMEM; /* can't do it */ @@ -793,7 +804,7 @@ build_rq_buffer(struct rqelement *rqe, struct plex *plex) /* Initialize the buf struct */ /* copy these flags from user bp */ bp->b_flags = ubp->b_flags & (B_NOCACHE | B_ASYNC); - bp->b_ioflags = ubp->b_ioflags & BIO_ORDERED; + bp->b_io.bio_flags = ubp->b_io.bio_flags & BIO_ORDERED; bp->b_iocmd = ubp->b_iocmd; BUF_LOCKINIT(bp); /* get a lock for the buffer */ BUF_LOCK(bp, LK_EXCLUSIVE); /* and lock it */ @@ -808,9 +819,8 @@ build_rq_buffer(struct rqelement *rqe, struct plex *plex) * obviously doesn't include drive information * when the drive is dead. */ - if ((rqe->flags & XFR_BAD_SUBDISK) == 0) { /* subdisk is accessible, */ + if ((rqe->flags & XFR_BAD_SUBDISK) == 0) /* subdisk is accessible, */ bp->b_dev = DRIVE[rqe->driveno].dev; /* drive device */ - } bp->b_blkno = rqe->sdoffset + sd->driveoffset; /* start address */ bp->b_bcount = rqe->buflen << DEV_BSHIFT; /* number of bytes to transfer */ bp->b_resid = bp->b_bcount; /* and it's still all waiting */ @@ -855,7 +865,7 @@ abortrequest(struct request *rq, int error) { struct buf *bp = rq->bp; /* user buffer */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bp->b_error = error; freerq(rq); /* free everything we're doing */ bufdone(bp); @@ -893,12 +903,12 @@ sdio(struct buf *bp) if (drive->state != drive_up) { if (sd->state >= sd_crashed) { - if (bp->b_iocmd == BIO_READ) /* reading, */ - set_sd_state(sd->sdno, sd_crashed, setstate_force); - else if (bp->b_iocmd == BIO_WRITE) /* writing, */ + if (bp->b_iocmd == BIO_WRITE) /* writing, */ set_sd_state(sd->sdno, sd_stale, setstate_force); + else + set_sd_state(sd->sdno, sd_crashed, setstate_force); } - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bp->b_error = EIO; bufdone(bp); return; @@ -908,7 +918,7 @@ sdio(struct buf *bp) * to get the I/O performed. */ if (sd->state < sd_empty) { /* nothing to talk to, */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bp->b_error = EIO; bufdone(bp); return; @@ -916,7 +926,7 @@ sdio(struct buf *bp) /* Get a buffer */ sbp = (struct sdbuf *) Malloc(sizeof(struct sdbuf)); if (sbp == NULL) { - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; bp->b_error = ENOMEM; bufdone(bp); return; @@ -998,7 +1008,7 @@ vinum_bounds_check(struct buf *bp, struct volume *vol) && (bp->b_iocmd == BIO_WRITE) /* and it's a write */ && (!vol->flags & (VF_WLABEL | VF_LABELLING))) { /* and we're not allowed to write the label */ bp->b_error = EROFS; /* read-only */ - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; return -1; } if (size == 0) /* no transfer specified, */ @@ -1015,7 +1025,7 @@ vinum_bounds_check(struct buf *bp, struct volume *vol) size = maxsize - bp->b_blkno; if (size <= 0) { /* nothing to transfer */ bp->b_error = EINVAL; - bp->b_ioflags |= BIO_ERROR; + bp->b_io.bio_flags |= BIO_ERROR; return -1; } bp->b_bcount = size << DEV_BSHIFT; @@ -1079,3 +1089,7 @@ deallocrqg(struct rqgroup *rqg) } Free(rqg); } + +/* Local Variables: */ +/* fill-column: 50 */ +/* End: */ |