Date: Thu, 2 Sep 2010 19:40:28 +0000 (UTC) From: "Justin T. Gibbs" <gibbs@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r212160 - in head/sys: cam/ata cam/scsi cddl/contrib/opensolaris/uts/common/fs/zfs geom geom/sched kern sys Message-ID: <201009021940.o82JeS8M017537@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: gibbs Date: Thu Sep 2 19:40:28 2010 New Revision: 212160 URL: http://svn.freebsd.org/changeset/base/212160 Log: Correct bioq_disksort so that bioq_insert_tail() offers barrier semantic. Add the BIO_ORDERED flag for struct bio and update bio clients to use it. The barrier semantics of bioq_insert_tail() were broken in two ways: o In bioq_disksort(), an added bio could be inserted at the head of the queue, even when a barrier was present, if the sort key for the new entry was less than that of the last queued barrier bio. o The last_offset used to generate the sort key for newly queued bios did not stay at the position of the barrier until either the barrier was de-queued, or a new barrier (which updates last_offset) was queued. When a barrier is in effect, we know that the disk will pass through the barrier position just before the "blocked bios" are released, so using the barrier's offset for last_offset is the optimal choice. sys/geom/sched/subr_disk.c: sys/kern/subr_disk.c: o Update last_offset in bioq_insert_tail(). o Only update last_offset in bioq_remove() if the removed bio is at the head of the queue (typically due to a call via bioq_takefirst()) and no barrier is active. o In bioq_disksort(), if we have a barrier (insert_point is non-NULL), set prev to the barrier and cur to it's next element. Now that last_offset is kept at the barrier position, this change isn't strictly necessary, but since we have to take a decision branch anyway, it does avoid one, no-op, loop iteration in the while loop that immediately follows. o In bioq_disksort(), bypass the normal sort for bios with the BIO_ORDERED attribute and instead insert them into the queue with bioq_insert_tail(). bioq_insert_tail() not only gives the desired command order during insertion, but also provides barrier semantics so that commands disksorted in the future cannot pass the just enqueued transaction. sys/sys/bio.h: Add BIO_ORDERED as bit 4 of the bio_flags field in struct bio. sys/cam/ata/ata_da.c: sys/cam/scsi/scsi_da.c Use an ordered command for SCSI/ATA-NCQ commands issued in response to bios with the BIO_ORDERED flag set. sys/cam/scsi/scsi_da.c Use an ordered tag when issuing a synchronize cache command. Wrap some lines to 80 columns. sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c sys/geom/geom_io.c Mark bios with the BIO_FLUSH command as BIO_ORDERED. Sponsored by: Spectra Logic Corporation MFC after: 1 month Modified: head/sys/cam/ata/ata_da.c head/sys/cam/scsi/scsi_da.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c head/sys/geom/geom_io.c head/sys/geom/sched/subr_disk.c head/sys/kern/subr_disk.c head/sys/sys/bio.h Modified: head/sys/cam/ata/ata_da.c ============================================================================== --- head/sys/cam/ata/ata_da.c Thu Sep 2 18:22:06 2010 (r212159) +++ head/sys/cam/ata/ata_da.c Thu Sep 2 19:40:28 2010 (r212160) @@ -874,7 +874,8 @@ adastart(struct cam_periph *periph, unio } bioq_remove(&softc->bio_queue, bp); - if ((softc->flags & ADA_FLAG_NEED_OTAG) != 0) { + if ((bp->bio_flags & BIO_ORDERED) != 0 + || (softc->flags & ADA_FLAG_NEED_OTAG) != 0) { softc->flags &= ~ADA_FLAG_NEED_OTAG; softc->ordered_tag_count++; tag_code = 0; Modified: head/sys/cam/scsi/scsi_da.c ============================================================================== --- head/sys/cam/scsi/scsi_da.c Thu Sep 2 18:22:06 2010 (r212159) +++ head/sys/cam/scsi/scsi_da.c Thu Sep 2 19:40:28 2010 (r212160) @@ -1354,7 +1354,8 @@ dastart(struct cam_periph *periph, union bioq_remove(&softc->bio_queue, bp); - if ((softc->flags & DA_FLAG_NEED_OTAG) != 0) { + if ((bp->bio_flags & BIO_ORDERED) != 0 + || (softc->flags & DA_FLAG_NEED_OTAG) != 0) { softc->flags &= ~DA_FLAG_NEED_OTAG; softc->ordered_tag_count++; tag_code = MSG_ORDERED_Q_TAG; @@ -1368,7 +1369,8 @@ dastart(struct cam_periph *periph, union /*retries*/da_retry_count, /*cbfcnp*/dadone, /*tag_action*/tag_code, - /*read_op*/bp->bio_cmd == BIO_READ, + /*read_op*/bp->bio_cmd + == BIO_READ, /*byte2*/0, softc->minimum_cmd_size, /*lba*/bp->bio_pblkno, @@ -1377,17 +1379,24 @@ dastart(struct cam_periph *periph, union /*data_ptr*/ bp->bio_data, /*dxfer_len*/ bp->bio_bcount, /*sense_len*/SSD_FULL_SIZE, - /*timeout*/da_default_timeout*1000); + da_default_timeout * 1000); break; case BIO_FLUSH: + /* + * BIO_FLUSH doesn't currently communicate + * range data, so we synchronize the cache + * over the whole disk. We also force + * ordered tag semantics the flush applies + * to all previously queued I/O. + */ scsi_synchronize_cache(&start_ccb->csio, /*retries*/1, /*cbfcnp*/dadone, - MSG_SIMPLE_Q_TAG, - /*begin_lba*/0,/* Cover the whole disk */ + MSG_ORDERED_Q_TAG, + /*begin_lba*/0, /*lb_count*/0, SSD_FULL_SIZE, - /*timeout*/da_default_timeout*1000); + da_default_timeout*1000); break; } start_ccb->ccb_h.ccb_state = DA_CCB_BUFFER_IO; Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Thu Sep 2 18:22:06 2010 (r212159) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Thu Sep 2 19:40:28 2010 (r212160) @@ -598,6 +598,7 @@ sendreq: break; case ZIO_TYPE_IOCTL: bp->bio_cmd = BIO_FLUSH; + bp->bio_flags |= BIO_ORDERED; bp->bio_data = NULL; bp->bio_offset = cp->provider->mediasize; bp->bio_length = 0; Modified: head/sys/geom/geom_io.c ============================================================================== --- head/sys/geom/geom_io.c Thu Sep 2 18:22:06 2010 (r212159) +++ head/sys/geom/geom_io.c Thu Sep 2 19:40:28 2010 (r212160) @@ -265,6 +265,7 @@ g_io_flush(struct g_consumer *cp) g_trace(G_T_BIO, "bio_flush(%s)", cp->provider->name); bp = g_alloc_bio(); bp->bio_cmd = BIO_FLUSH; + bp->bio_flags |= BIO_ORDERED; bp->bio_done = NULL; bp->bio_attribute = NULL; bp->bio_offset = cp->provider->mediasize; Modified: head/sys/geom/sched/subr_disk.c ============================================================================== --- head/sys/geom/sched/subr_disk.c Thu Sep 2 18:22:06 2010 (r212159) +++ head/sys/geom/sched/subr_disk.c Thu Sep 2 19:40:28 2010 (r212160) @@ -86,7 +86,7 @@ __FBSDID("$FreeBSD$"); * bioq_remove() remove a generic element from the queue, act as * bioq_takefirst() if invoked on the head of the queue. * - * The semantic of these methods is the same of the operations + * The semantic of these methods is the same as the operations * on the underlying TAILQ, but with additional guarantees on * subsequent bioq_disksort() calls. E.g. bioq_insert_tail() * can be useful for making sure that all previous ops are flushed @@ -115,10 +115,10 @@ void gs_bioq_remove(struct bio_queue_head *head, struct bio *bp) { - if (bp == TAILQ_FIRST(&head->queue)) - head->last_offset = bp->bio_offset + bp->bio_length; - - if (bp == head->insert_point) + if (head->insert_point == NULL) { + if (bp == TAILQ_FIRST(&head->queue)) + head->last_offset = bp->bio_offset + bp->bio_length; + } else if (bp == head->insert_point) head->insert_point = NULL; TAILQ_REMOVE(&head->queue, bp, bio_queue); @@ -137,7 +137,8 @@ void gs_bioq_insert_head(struct bio_queue_head *head, struct bio *bp) { - head->last_offset = bp->bio_offset; + if (head->insert_point == NULL) + head->last_offset = bp->bio_offset; TAILQ_INSERT_HEAD(&head->queue, bp, bio_queue); } @@ -147,6 +148,7 @@ gs_bioq_insert_tail(struct bio_queue_hea TAILQ_INSERT_TAIL(&head->queue, bp, bio_queue); head->insert_point = bp; + head->last_offset = bp->bio_offset; } struct bio * @@ -189,13 +191,28 @@ gs_bioq_bio_key(struct bio_queue_head *h void gs_bioq_disksort(struct bio_queue_head *head, struct bio *bp) { - struct bio *cur, *prev = NULL; - uoff_t key = gs_bioq_bio_key(head, bp); + struct bio *cur, *prev; + uoff_t key; + if ((bp->bio_flags & BIO_ORDERED) != 0) { + /* + * Ordered transactions can only be dispatched + * after any currently queued transactions. They + * also have barrier semantics - no transactions + * queued in the future can pass them. + */ + gs_bioq_insert_tail(head, bp); + return; + } + + prev = NULL; + key = gs_bioq_bio_key(head, bp); cur = TAILQ_FIRST(&head->queue); - if (head->insert_point) - cur = head->insert_point; + if (head->insert_point) { + prev = head->insert_point; + cur = TAILQ_NEXT(head->insert_point, bio_queue); + } while (cur != NULL && key >= gs_bioq_bio_key(head, cur)) { prev = cur; Modified: head/sys/kern/subr_disk.c ============================================================================== --- head/sys/kern/subr_disk.c Thu Sep 2 18:22:06 2010 (r212159) +++ head/sys/kern/subr_disk.c Thu Sep 2 19:40:28 2010 (r212160) @@ -127,7 +127,7 @@ disk_err(struct bio *bp, const char *wha * bioq_remove() remove a generic element from the queue, act as * bioq_takefirst() if invoked on the head of the queue. * - * The semantic of these methods is the same of the operations + * The semantic of these methods is the same as the operations * on the underlying TAILQ, but with additional guarantees on * subsequent bioq_disksort() calls. E.g. bioq_insert_tail() * can be useful for making sure that all previous ops are flushed @@ -156,10 +156,10 @@ void bioq_remove(struct bio_queue_head *head, struct bio *bp) { - if (bp == TAILQ_FIRST(&head->queue)) - head->last_offset = bp->bio_offset + bp->bio_length; - - if (bp == head->insert_point) + if (head->insert_point == NULL) { + if (bp == TAILQ_FIRST(&head->queue)) + head->last_offset = bp->bio_offset + bp->bio_length; + } else if (bp == head->insert_point) head->insert_point = NULL; TAILQ_REMOVE(&head->queue, bp, bio_queue); @@ -178,7 +178,8 @@ void bioq_insert_head(struct bio_queue_head *head, struct bio *bp) { - head->last_offset = bp->bio_offset; + if (head->insert_point == NULL) + head->last_offset = bp->bio_offset; TAILQ_INSERT_HEAD(&head->queue, bp, bio_queue); } @@ -188,6 +189,7 @@ bioq_insert_tail(struct bio_queue_head * TAILQ_INSERT_TAIL(&head->queue, bp, bio_queue); head->insert_point = bp; + head->last_offset = bp->bio_offset; } struct bio * @@ -230,13 +232,28 @@ bioq_bio_key(struct bio_queue_head *head void bioq_disksort(struct bio_queue_head *head, struct bio *bp) { - struct bio *cur, *prev = NULL; - uoff_t key = bioq_bio_key(head, bp); + struct bio *cur, *prev; + uoff_t key; + if ((bp->bio_flags & BIO_ORDERED) != 0) { + /* + * Ordered transactions can only be dispatched + * after any currently queued transactions. They + * also have barrier semantics - no transactions + * queued in the future can pass them. + */ + bioq_insert_tail(head, bp); + return; + } + + prev = NULL; + key = bioq_bio_key(head, bp); cur = TAILQ_FIRST(&head->queue); - if (head->insert_point) - cur = head->insert_point; + if (head->insert_point) { + prev = head->insert_point; + cur = TAILQ_NEXT(head->insert_point, bio_queue); + } while (cur != NULL && key >= bioq_bio_key(head, cur)) { prev = cur; Modified: head/sys/sys/bio.h ============================================================================== --- head/sys/sys/bio.h Thu Sep 2 18:22:06 2010 (r212159) +++ head/sys/sys/bio.h Thu Sep 2 19:40:28 2010 (r212160) @@ -54,6 +54,7 @@ #define BIO_ERROR 0x01 #define BIO_DONE 0x02 #define BIO_ONQUEUE 0x04 +#define BIO_ORDERED 0x08 #ifdef _KERNEL struct disk;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009021940.o82JeS8M017537>