Skip to contentSkip to conversation

block: convert to REQ_FLUSH/FUA

RFC PATCH In review Tejun Heo 12 participants 52 messages Aug 16, 2010 – Nov 10, 2010 1 message not in the archive

Subsystems (16)
Documentation VFS SCSI USB +12 more subsystems
lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

RFC PATCH 1/1 block: convert to REQ_FLUSH/FUA

Hello,

This patchset contains five patches to convert block drivers implementing REQ_HARDBARRIER to support REQ_FLUSH/FUA.

0001-block-loop-implement-REQ_FLUSH-FUA-support.patch 0002-virtio_blk-implement-REQ_FLUSH-FUA-support.patch 0003-lguest-replace-VIRTIO_F_BARRIER-support-with-VIRTIO_.patch 0004-md-implment-REQ_FLUSH-FUA-support.patch 0005-dm-implement-REQ_FLUSH-FUA-support.patch

I'm fairly sure about conversions 0001-0003. 0004 should be okay although multipath wasn't tested. 0005, I'm not quite sure about. It works fine for the tests I've done but there are many other targets and code paths that I didn't test. So, please be careful with the last patch. I think it would be best to route the last two through the respective md/dm trees after the core part is merged and pulled into those trees.

The nice thing about the conversion is that in many cases it replaces postflush with FUA writes which can be handled by request queues lower in the chain. For md/dm, this replaces an array wide cacheflush with FUA writes to only affected member devices.

After this patchset, the followings remain to be converted.

* blktrace

* scsi_error.c for some reason tests REQ_HARDBARRIER. I think this part can be dropped altogether but am not sure.

* drbd and xen. I have no idea.

These patches are on top of

block#for-2.6.36-post (c047ab2dddeeafbd6f7c00e45a13a5c4da53ea0b)
+ block-replace-barrier-with-sequenced-flush patchset[1]
+ block-fix-incorrect-bio-request-flag-conversion-in-md patch[2]

and available in the following git tree.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git flush-fua

and contain the following changes.

 Documentation/lguest/lguest.c   |   36 +++-----
 drivers/block/loop.c            |   18 ++--
 drivers/block/virtio_blk.c      |   26 ++---
 drivers/md/dm-crypt.c           |    2 
 drivers/md/dm-io.c              |   20 ----
 drivers/md/dm-log.c             |    2 
 drivers/md/dm-raid1.c           |    8 -
 drivers/md/dm-region-hash.c     |   16 +--
 drivers/md/dm-snap-persistent.c |    2 
 drivers/md/dm-snap.c            |    6 -
 drivers/md/dm-stripe.c          |    2 
 drivers/md/dm.c                 |  180 ++++++++++++++++++++--------------------
 drivers/md/linear.c             |    4 
 drivers/md/md.c                 |  117 +++++---------------------
 drivers/md/md.h                 |   23 +----
 drivers/md/multipath.c          |    4 
 drivers/md/raid0.c              |    4 
 drivers/md/raid1.c              |  175 +++++++++++++-------------------------
 drivers/md/raid1.h              |    2 
 drivers/md/raid10.c             |    7 -
 drivers/md/raid5.c              |   38 ++++----
 drivers/md/raid5.h              |    1 
 include/linux/virtio_blk.h      |    6 +
 23 files changed, 278 insertions(+), 421 deletions(-)

--
tejun

[1] http://thread.gmane.org/gmane.linux.kernel/1022363
[2] http://thread.gmane.org/gmane.linux.kernel/1023435

author
#1 lore raw

PATCH 1/5 block/loop: implement REQ_FLUSH/FUA support

From: Tejun Heo <[email protected]>

Deprecate REQ_HARDBARRIER and implement REQ_FLUSH/FUA instead. Also, instead of checking file->f_op->fsync() directly, look at the value of vfs_fsync() and ignore -EINVAL return.

---
 drivers/block/loop.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)
drivers/block/loop.c +9 -9
index 953d1e1..5d27bc6 100644
@@ -477,17 +477,17 @@ static int do_bio_filebacked(struct loop_device *lo, struct bio *bio)
 	pos = ((loff_t) bio->bi_sector << 9) + lo->lo_offset;
 
 	if (bio_rw(bio) == WRITE) {
-		bool barrier = (bio->bi_rw & REQ_HARDBARRIER);
 		struct file *file = lo->lo_backing_file;
 
-		if (barrier) {
-			if (unlikely(!file->f_op->fsync)) {
-				ret = -EOPNOTSUPP;
-				goto out;
-			}
+		/* REQ_HARDBARRIER is deprecated */
+		if (bio->bi_rw & REQ_HARDBARRIER) {
+			ret = -EOPNOTSUPP;
+			goto out;
+		}
 
+		if (bio->bi_rw & REQ_FLUSH) {
 			ret = vfs_fsync(file, 0);
-			if (unlikely(ret)) {
+			if (unlikely(ret && ret != -EINVAL)) {
 				ret = -EIO;
 				goto out;
 			}
@@ -495,9 +495,9 @@ static int do_bio_filebacked(struct loop_device *lo, struct bio *bio)
 
 		ret = lo_send(lo, bio, pos);
 
-		if (barrier && !ret) {
+		if ((bio->bi_rw & REQ_FUA) && !ret) {
 			ret = vfs_fsync(file, 0);
-			if (unlikely(ret))
+			if (unlikely(ret && ret != -EINVAL))
 				ret = -EIO;
 		}
 	} else
Signature · 1 line
1.7.1
author
#2 lore raw

PATCH 1/5 block/loop: implement REQ_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

Deprecate REQ_HARDBARRIER and implement REQ_FLUSH/FUA instead. Also, instead of checking file->f_op->fsync() directly, look at the value of vfs_fsync() and ignore -EINVAL return.

---
 drivers/block/loop.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)
drivers/block/loop.c +9 -9
index 953d1e1..5d27bc6 100644
@@ -477,17 +477,17 @@ static int do_bio_filebacked(struct loop_device *lo, struct bio *bio)
 	pos = ((loff_t) bio->bi_sector << 9) + lo->lo_offset;
 
 	if (bio_rw(bio) == WRITE) {
-		bool barrier = (bio->bi_rw & REQ_HARDBARRIER);
 		struct file *file = lo->lo_backing_file;
 
-		if (barrier) {
-			if (unlikely(!file->f_op->fsync)) {
-				ret = -EOPNOTSUPP;
-				goto out;
-			}
+		/* REQ_HARDBARRIER is deprecated */
+		if (bio->bi_rw & REQ_HARDBARRIER) {
+			ret = -EOPNOTSUPP;
+			goto out;
+		}
 
+		if (bio->bi_rw & REQ_FLUSH) {
 			ret = vfs_fsync(file, 0);
-			if (unlikely(ret)) {
+			if (unlikely(ret && ret != -EINVAL)) {
 				ret = -EIO;
 				goto out;
 			}
@@ -495,9 +495,9 @@ static int do_bio_filebacked(struct loop_device *lo, struct bio *bio)
 
 		ret = lo_send(lo, bio, pos);
 
-		if (barrier && !ret) {
+		if ((bio->bi_rw & REQ_FUA) && !ret) {
 			ret = vfs_fsync(file, 0);
-			if (unlikely(ret))
+			if (unlikely(ret && ret != -EINVAL))
 				ret = -EIO;
 		}
 	} else
Signature · 1 line
1.7.1
author
#3 lore raw

PATCH 2/5 virtio_blk: implement REQ_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

Remove now unused REQ_HARDBARRIER support and implement REQ_FLUSH/FUA support instead. A new feature flag VIRTIO_BLK_F_FUA is added to indicate the support for FUA.

Cc Michael S. Tsirkin <[email protected]>
---
 drivers/block/virtio_blk.c |   26 ++++++++++++--------------
 include/linux/virtio_blk.h |    6 +++++-
 2 files changed, 17 insertions(+), 15 deletions(-)
drivers/block/virtio_blk.c +12 -14
index d10b635..ed0fb7d 100644
@@ -128,9 +128,6 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
 		}
 	}
 
-	if (vbr->req->cmd_flags & REQ_HARDBARRIER)
-		vbr->out_hdr.type |= VIRTIO_BLK_T_BARRIER;
-
 	sg_set_buf(&vblk->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr));
 
 	/*
@@ -157,6 +154,8 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
 		if (rq_data_dir(vbr->req) == WRITE) {
 			vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
 			out += num;
+			if (req->cmd_flags & REQ_FUA)
+				vbr->out_hdr.type |= VIRTIO_BLK_T_FUA;
 		} else {
 			vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
 			in += num;
@@ -307,6 +306,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
 {
 	struct virtio_blk *vblk;
 	struct request_queue *q;
+	unsigned int flush;
 	int err;
 	u64 cap;
 	u32 v, blk_size, sg_elems, opt_io_size;
@@ -388,15 +388,13 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
 	vblk->disk->driverfs_dev = &vdev->dev;
 	index++;
 
-	/*
-	 * If the FLUSH feature is supported we do have support for
-	 * flushing a volatile write cache on the host.  Use that to
-	 * implement write barrier support; otherwise, we must assume
-	 * that the host does not perform any kind of volatile write
-	 * caching.
-	 */
+	/* configure queue flush support */
+	flush = 0;
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH))
-		blk_queue_flush(q, REQ_FLUSH);
+		flush |= REQ_FLUSH;
+	if (virtio_has_feature(vdev, VIRTIO_BLK_F_FUA))
+		flush |= REQ_FUA;
+	blk_queue_flush(q, flush);
 
 	/* If disk is read-only in the host, the guest should obey */
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_RO))
@@ -515,9 +513,9 @@ static const struct virtio_device_id id_table[] = {
 };
 
 static unsigned int features[] = {
-	VIRTIO_BLK_F_BARRIER, VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX,
-	VIRTIO_BLK_F_GEOMETRY, VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
-	VIRTIO_BLK_F_SCSI, VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY
+	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
+	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
+	VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_FUA,
 };
 
 /*
include/linux/virtio_blk.h +5 -1
index 167720d..f453f3c 100644
@@ -16,6 +16,7 @@
 #define VIRTIO_BLK_F_SCSI	7	/* Supports scsi command passthru */
 #define VIRTIO_BLK_F_FLUSH	9	/* Cache flush command support */
 #define VIRTIO_BLK_F_TOPOLOGY	10	/* Topology information is available */
+#define VIRTIO_BLK_F_FUA	11	/* Forced Unit Access write support */
 
 #define VIRTIO_BLK_ID_BYTES	20	/* ID string length */
 
@@ -70,7 +71,10 @@ struct virtio_blk_config {
 #define VIRTIO_BLK_T_FLUSH	4
 
 /* Get device ID command */
-#define VIRTIO_BLK_T_GET_ID    8
+#define VIRTIO_BLK_T_GET_ID	8
+
+/* FUA command */
+#define VIRTIO_BLK_T_FUA	16
 
 /* Barrier before this op. */
 #define VIRTIO_BLK_T_BARRIER	0x80000000
Signature · 1 line
1.7.1
#4 lore raw

Re: [PATCH 2/5] virtio_blk: implement REQ_FLUSH/FUA support

Quoted 5 lines · Tejun Heo

From: Tejun Heo <[email protected]>

Remove now unused REQ_HARDBARRIER support and implement REQ_FLUSH/FUA support instead. A new feature flag VIRTIO_BLK_F_FUA is added to indicate the support for FUA.

I'm not sure it's worth it. The pure REQ_FLUSH path works not and is
well tested with kvm/qemu. We can still easily add a FUA bit, and
even a pre-flush bit if the protocol roundtrips matter in real life benchmarking.

author
#5 lore raw

Re: [PATCH 2/5] virtio_blk: implement REQ_FLUSH/FUA support

Hello,

Quoted 11 lines · Christoph Hellwig
Quoted 5 lines · Tejun Heo

From: Tejun Heo <[email protected]>

Remove now unused REQ_HARDBARRIER support and implement REQ_FLUSH/FUA support instead. A new feature flag VIRTIO_BLK_F_FUA is added to indicate the support for FUA.

I'm not sure it's worth it. The pure REQ_FLUSH path works not and is
well tested with kvm/qemu. We can still easily add a FUA bit, and
even a pre-flush bit if the protocol roundtrips matter in real life benchmarking.

Hmmm... the underlying storage could be md/dm RAIDs in which case FUA should be cheaper than FLUSH.

Thanks.

Signature · 1 line
tejun
#6 lore raw

Re: [PATCH 2/5] virtio_blk: implement REQ_FLUSH/FUA support

Quoted 11 lines · Tejun Heo
Quoted 8 lines
Quoted 3 lines

Remove now unused REQ_HARDBARRIER support and implement REQ_FLUSH/FUA support instead. A new feature flag VIRTIO_BLK_F_FUA is added to indicate the support for FUA.

I'm not sure it's worth it. The pure REQ_FLUSH path works not and is
well tested with kvm/qemu. We can still easily add a FUA bit, and
even a pre-flush bit if the protocol roundtrips matter in real life benchmarking.

Hmmm... the underlying storage could be md/dm RAIDs in which case FUA should be cheaper than FLUSH.

If someone ever wrote a virtio-blk backend that sits directly ontop of the Linux block layer that would be true. Of the five known virtio-blk backends all operate on normal files using the Posix I/O APIs, or the Linux aio API (optionally in qemu) or in-kernel vfs_read/vfs_write (vhost-blk).

Given how little testing lguest gets compared to qemu I really don't want a protocol addition for it unless it really buys us something. Once we're done with this barrier conversion I plan into benchmarking FUA and a pre-flush tag on the command for virtio in real life setups, and see if it actually buys us anything.

author
#7 lore raw

Re: [PATCH 2/5] virtio_blk: implement REQ_FLUSH/FUA support

Hello,

Quoted 8 lines · Christoph Hellwig
Quoted 2 lines

Hmmm... the underlying storage could be md/dm RAIDs in which case FUA should be cheaper than FLUSH.

If someone ever wrote a virtio-blk backend that sits directly ontop of the Linux block layer that would be true. Of the five known virtio-blk backends all operate on normal files using the Posix I/O APIs, or the Linux aio API (optionally in qemu) or in-kernel vfs_read/vfs_write (vhost-blk).

Right.

Quoted 5 lines

Given how little testing lguest gets compared to qemu I really don't want a protocol addition for it unless it really buys us something. Once we're done with this barrier conversion I plan into benchmarking FUA and a pre-flush tag on the command for virtio in real life setups, and see if it actually buys us anything.

Hmmm... yeah, we can drop it. Michael, what do you think?

Thanks.

Signature · 1 line
tejun
#8 lore raw

Re: [PATCH 2/5] virtio_blk: implement REQ_FLUSH/FUA support

In reply to Christoph Hellwig
Quoted 5 lines · Christoph Hellwig

Given how little testing lguest gets compared to qemu I really don't want a protocol addition for it unless it really buys us something. Once we're done with this barrier conversion I plan into benchmarking FUA and a pre-flush tag on the command for virtio in real life setups, and see if it actually buys us anything.

Absolutely. Lguest should follow, not lead!

Rusty.

Acked-by on v1 2/5
#9 lore raw

Re: [PATCH 2/5] virtio_blk: implement REQ_FLUSH/FUA support

In reply to Tejun Heo
Quoted 8 lines · Tejun Heo

From: Tejun Heo <[email protected]>

Remove now unused REQ_HARDBARRIER support and implement REQ_FLUSH/FUA support instead. A new feature flag VIRTIO_BLK_F_FUA is added to indicate the support for FUA.

Cc Michael S. Tsirkin <[email protected]>

And also for the lguest-specific patch.

Thanks!
Rusty.

author
#10 lore raw

Re: [PATCH 2/5] virtio_blk: implement REQ_FLUSH/FUA support

Quoted 7 lines · Rusty Russell
Quoted 6 lines · Tejun Heo

From: Tejun Heo <[email protected]>

Remove now unused REQ_HARDBARRIER support and implement REQ_FLUSH/FUA support instead. A new feature flag VIRTIO_BLK_F_FUA is added to indicate the support for FUA.

Quoted 6 lines
Quoted 2 lines
Cc Michael S. Tsirkin <[email protected]>

And also for the lguest-specific patch.

Thanks. Acked-by's added.

Signature · 1 line
tejun
author
#11 lore raw

PATCH 2/5 virtio_blk: drop REQ_HARDBARRIER support

In reply to Tejun Heo

Remove now unused REQ_HARDBARRIER support. virtio_blk already supports REQ_FLUSH and the usefulness of REQ_FUA for virtio_blk is questionable at this point, so there's nothing else to do to support new REQ_FLUSH/FUA interface.

Cc Michael S. Tsirkin <[email protected]>
Cc Christoph Hellwig <[email protected]>

---
REQ_FUA support dropped as suggested by Christoph.

 drivers/block/virtio_blk.c |   17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)
drivers/block/virtio_blk.c +4 -13
Index: block/drivers/block/virtio_blk.c
===================================================================
@@ -128,9 +128,6 @@ static bool do_req(struct request_queue
 		}
 	}
 
-	if (vbr->req->cmd_flags & REQ_HARDBARRIER)
-		vbr->out_hdr.type |= VIRTIO_BLK_T_BARRIER;
-
 	sg_set_buf(&vblk->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr));
 
 	/*
@@ -388,13 +385,7 @@ static int __devinit virtblk_probe(struc
 	vblk->disk->driverfs_dev = &vdev->dev;
 	index++;
 
-	/*
-	 * If the FLUSH feature is supported we do have support for
-	 * flushing a volatile write cache on the host.  Use that to
-	 * implement write barrier support; otherwise, we must assume
-	 * that the host does not perform any kind of volatile write
-	 * caching.
-	 */
+	/* configure queue flush support */
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH))
 		blk_queue_flush(q, REQ_FLUSH);
 
@@ -515,9 +506,9 @@ static const struct virtio_device_id id_
 };
 
 static unsigned int features[] = {
-	VIRTIO_BLK_F_BARRIER, VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX,
-	VIRTIO_BLK_F_GEOMETRY, VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
-	VIRTIO_BLK_F_SCSI, VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY
+	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
+	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
+	VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY
 };
 
 /*
author
#12 lore raw

PATCH 2/5 virtio_blk: implement REQ_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

Remove now unused REQ_HARDBARRIER support and implement REQ_FLUSH/FUA support instead. A new feature flag VIRTIO_BLK_F_FUA is added to indicate the support for FUA.

Cc Michael S. Tsirkin <[email protected]>
---
 drivers/block/virtio_blk.c |   26 ++++++++++++--------------
 include/linux/virtio_blk.h |    6 +++++-
 2 files changed, 17 insertions(+), 15 deletions(-)
drivers/block/virtio_blk.c +12 -14
index d10b635..ed0fb7d 100644
@@ -128,9 +128,6 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
 		}
 	}
 
-	if (vbr->req->cmd_flags & REQ_HARDBARRIER)
-		vbr->out_hdr.type |= VIRTIO_BLK_T_BARRIER;
-
 	sg_set_buf(&vblk->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr));
 
 	/*
@@ -157,6 +154,8 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
 		if (rq_data_dir(vbr->req) == WRITE) {
 			vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
 			out += num;
+			if (req->cmd_flags & REQ_FUA)
+				vbr->out_hdr.type |= VIRTIO_BLK_T_FUA;
 		} else {
 			vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
 			in += num;
@@ -307,6 +306,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
 {
 	struct virtio_blk *vblk;
 	struct request_queue *q;
+	unsigned int flush;
 	int err;
 	u64 cap;
 	u32 v, blk_size, sg_elems, opt_io_size;
@@ -388,15 +388,13 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
 	vblk->disk->driverfs_dev = &vdev->dev;
 	index++;
 
-	/*
-	 * If the FLUSH feature is supported we do have support for
-	 * flushing a volatile write cache on the host.  Use that to
-	 * implement write barrier support; otherwise, we must assume
-	 * that the host does not perform any kind of volatile write
-	 * caching.
-	 */
+	/* configure queue flush support */
+	flush = 0;
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH))
-		blk_queue_flush(q, REQ_FLUSH);
+		flush |= REQ_FLUSH;
+	if (virtio_has_feature(vdev, VIRTIO_BLK_F_FUA))
+		flush |= REQ_FUA;
+	blk_queue_flush(q, flush);
 
 	/* If disk is read-only in the host, the guest should obey */
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_RO))
@@ -515,9 +513,9 @@ static const struct virtio_device_id id_table[] = {
 };
 
 static unsigned int features[] = {
-	VIRTIO_BLK_F_BARRIER, VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX,
-	VIRTIO_BLK_F_GEOMETRY, VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
-	VIRTIO_BLK_F_SCSI, VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY
+	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
+	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
+	VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_FUA,
 };
 
 /*
include/linux/virtio_blk.h +5 -1
index 167720d..f453f3c 100644
@@ -16,6 +16,7 @@
 #define VIRTIO_BLK_F_SCSI	7	/* Supports scsi command passthru */
 #define VIRTIO_BLK_F_FLUSH	9	/* Cache flush command support */
 #define VIRTIO_BLK_F_TOPOLOGY	10	/* Topology information is available */
+#define VIRTIO_BLK_F_FUA	11	/* Forced Unit Access write support */
 
 #define VIRTIO_BLK_ID_BYTES	20	/* ID string length */
 
@@ -70,7 +71,10 @@ struct virtio_blk_config {
 #define VIRTIO_BLK_T_FLUSH	4
 
 /* Get device ID command */
-#define VIRTIO_BLK_T_GET_ID    8
+#define VIRTIO_BLK_T_GET_ID	8
+
+/* FUA command */
+#define VIRTIO_BLK_T_FUA	16
 
 /* Barrier before this op. */
 #define VIRTIO_BLK_T_BARRIER	0x80000000
Signature · 1 line
1.7.1
author
#13 lore raw

PATCH 3/5 lguest: replace VIRTIO_F_BARRIER support with VIRTIO_F_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

VIRTIO_F_BARRIER is deprecated. Replace it with VIRTIO_F_FLUSH/FUA support.

Cc Rusty Russell <[email protected]>
---
 Documentation/lguest/lguest.c |   36 ++++++++++++++++--------------------
 1 files changed, 16 insertions(+), 20 deletions(-)
Documentation/lguest/lguest.c +16 -20
index e9ce3c5..3be47d4 100644
@@ -1639,15 +1639,6 @@ static void blk_request(struct virtqueue *vq)
 	off = out->sector * 512;
 
 	/*
-	 * The block device implements "barriers", where the Guest indicates
-	 * that it wants all previous writes to occur before this write.  We
-	 * don't have a way of asking our kernel to do a barrier, so we just
-	 * synchronize all the data in the file.  Pretty poor, no?
-	 */
-	if (out->type & VIRTIO_BLK_T_BARRIER)
-		fdatasync(vblk->fd);
-
-	/*
 	 * In general the virtio block driver is allowed to try SCSI commands.
 	 * It'd be nice if we supported eject, for example, but we don't.
 	 */
@@ -1679,6 +1670,19 @@ static void blk_request(struct virtqueue *vq)
 			/* Die, bad Guest, die. */
 			errx(1, "Write past end %llu+%u", off, ret);
 		}
+
+		/* Honor FUA by syncing everything. */
+		if (ret >= 0 && (out->type & VIRTIO_BLK_T_FUA)) {
+			ret = fdatasync(vblk->fd);
+			verbose("FUA fdatasync: %i\n", ret);
+		}
+
+		wlen = sizeof(*in);
+		*in = (ret >= 0 ? VIRTIO_BLK_S_OK : VIRTIO_BLK_S_IOERR);
+	} else if (out->type & VIRTIO_BLK_T_FLUSH) {
+		/* Flush */
+		ret = fdatasync(vblk->fd);
+		verbose("FLUSH fdatasync: %i\n", ret);
 		wlen = sizeof(*in);
 		*in = (ret >= 0 ? VIRTIO_BLK_S_OK : VIRTIO_BLK_S_IOERR);
 	} else {
@@ -1702,15 +1706,6 @@ static void blk_request(struct virtqueue *vq)
 		}
 	}
 
-	/*
-	 * OK, so we noted that it was pretty poor to use an fdatasync as a
-	 * barrier.  But Christoph Hellwig points out that we need a sync
-	 * *afterwards* as well: "Barriers specify no reordering to the front
-	 * or the back."  And Jens Axboe confirmed it, so here we are:
-	 */
-	if (out->type & VIRTIO_BLK_T_BARRIER)
-		fdatasync(vblk->fd);
-
 	/* Finished that request. */
 	add_used(vq, head, wlen);
 }
@@ -1735,8 +1730,9 @@ static void setup_block_file(const char *filename)
 	vblk->fd = open_or_die(filename, O_RDWR|O_LARGEFILE);
 	vblk->len = lseek64(vblk->fd, 0, SEEK_END);
 
-	/* We support barriers. */
-	add_feature(dev, VIRTIO_BLK_F_BARRIER);
+	/* We support FLUSH and FUA. */
+	add_feature(dev, VIRTIO_BLK_F_FLUSH);
+	add_feature(dev, VIRTIO_BLK_F_FUA);
 
 	/* Tell Guest how many sectors this device has. */
 	conf.capacity = cpu_to_le64(vblk->len / 512);
Signature · 1 line
1.7.1
author
#14 lore raw

Re: [PATCH 3/5] lguest: replace VIRTIO_F_BARRIER support with VIRTIO_F_FLUSH support

VIRTIO_F_BARRIER is deprecated. Replace it with VIRTIO_F_FLUSH support.

Cc Rusty Russell <[email protected]>
Cc Christoph Hellwig <[email protected]>

---
FUA support dropped as suggested by Christoph. Rusty, can you please ack this version too? I tested it with the updated virtio_blk and it works fine.

Thanks.

 Documentation/lguest/lguest.c |   29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)
Documentation/lguest/lguest.c +9 -20
Index: block/Documentation/lguest/lguest.c
===================================================================
@@ -1639,15 +1639,6 @@ static void blk_request(struct virtqueue
 	off = out->sector * 512;
 
 	/*
-	 * The block device implements "barriers", where the Guest indicates
-	 * that it wants all previous writes to occur before this write.  We
-	 * don't have a way of asking our kernel to do a barrier, so we just
-	 * synchronize all the data in the file.  Pretty poor, no?
-	 */
-	if (out->type & VIRTIO_BLK_T_BARRIER)
-		fdatasync(vblk->fd);
-
-	/*
 	 * In general the virtio block driver is allowed to try SCSI commands.
 	 * It'd be nice if we supported eject, for example, but we don't.
 	 */
@@ -1679,6 +1670,13 @@ static void blk_request(struct virtqueue
 			/* Die, bad Guest, die. */
 			errx(1, "Write past end %llu+%u", off, ret);
 		}
+
+		wlen = sizeof(*in);
+		*in = (ret >= 0 ? VIRTIO_BLK_S_OK : VIRTIO_BLK_S_IOERR);
+	} else if (out->type & VIRTIO_BLK_T_FLUSH) {
+		/* Flush */
+		ret = fdatasync(vblk->fd);
+		verbose("FLUSH fdatasync: %i\n", ret);
 		wlen = sizeof(*in);
 		*in = (ret >= 0 ? VIRTIO_BLK_S_OK : VIRTIO_BLK_S_IOERR);
 	} else {
@@ -1702,15 +1700,6 @@ static void blk_request(struct virtqueue
 		}
 	}
 
-	/*
-	 * OK, so we noted that it was pretty poor to use an fdatasync as a
-	 * barrier.  But Christoph Hellwig points out that we need a sync
-	 * *afterwards* as well: "Barriers specify no reordering to the front
-	 * or the back."  And Jens Axboe confirmed it, so here we are:
-	 */
-	if (out->type & VIRTIO_BLK_T_BARRIER)
-		fdatasync(vblk->fd);
-
 	/* Finished that request. */
 	add_used(vq, head, wlen);
 }
@@ -1735,8 +1724,8 @@ static void setup_block_file(const char
 	vblk->fd = open_or_die(filename, O_RDWR|O_LARGEFILE);
 	vblk->len = lseek64(vblk->fd, 0, SEEK_END);
 
-	/* We support barriers. */
-	add_feature(dev, VIRTIO_BLK_F_BARRIER);
+	/* We support FLUSH. */
+	add_feature(dev, VIRTIO_BLK_F_FLUSH);
 
 	/* Tell Guest how many sectors this device has. */
 	conf.capacity = cpu_to_le64(vblk->len / 512);
author
#15 lore raw

PATCH 3/5 lguest: replace VIRTIO_F_BARRIER support with VIRTIO_F_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

VIRTIO_F_BARRIER is deprecated. Replace it with VIRTIO_F_FLUSH/FUA support.

Cc Rusty Russell <[email protected]>
---
 Documentation/lguest/lguest.c |   36 ++++++++++++++++--------------------
 1 files changed, 16 insertions(+), 20 deletions(-)
Documentation/lguest/lguest.c +16 -20
index e9ce3c5..3be47d4 100644
@@ -1639,15 +1639,6 @@ static void blk_request(struct virtqueue *vq)
 	off = out->sector * 512;
 
 	/*
-	 * The block device implements "barriers", where the Guest indicates
-	 * that it wants all previous writes to occur before this write.  We
-	 * don't have a way of asking our kernel to do a barrier, so we just
-	 * synchronize all the data in the file.  Pretty poor, no?
-	 */
-	if (out->type & VIRTIO_BLK_T_BARRIER)
-		fdatasync(vblk->fd);
-
-	/*
 	 * In general the virtio block driver is allowed to try SCSI commands.
 	 * It'd be nice if we supported eject, for example, but we don't.
 	 */
@@ -1679,6 +1670,19 @@ static void blk_request(struct virtqueue *vq)
 			/* Die, bad Guest, die. */
 			errx(1, "Write past end %llu+%u", off, ret);
 		}
+
+		/* Honor FUA by syncing everything. */
+		if (ret >= 0 && (out->type & VIRTIO_BLK_T_FUA)) {
+			ret = fdatasync(vblk->fd);
+			verbose("FUA fdatasync: %i\n", ret);
+		}
+
+		wlen = sizeof(*in);
+		*in = (ret >= 0 ? VIRTIO_BLK_S_OK : VIRTIO_BLK_S_IOERR);
+	} else if (out->type & VIRTIO_BLK_T_FLUSH) {
+		/* Flush */
+		ret = fdatasync(vblk->fd);
+		verbose("FLUSH fdatasync: %i\n", ret);
 		wlen = sizeof(*in);
 		*in = (ret >= 0 ? VIRTIO_BLK_S_OK : VIRTIO_BLK_S_IOERR);
 	} else {
@@ -1702,15 +1706,6 @@ static void blk_request(struct virtqueue *vq)
 		}
 	}
 
-	/*
-	 * OK, so we noted that it was pretty poor to use an fdatasync as a
-	 * barrier.  But Christoph Hellwig points out that we need a sync
-	 * *afterwards* as well: "Barriers specify no reordering to the front
-	 * or the back."  And Jens Axboe confirmed it, so here we are:
-	 */
-	if (out->type & VIRTIO_BLK_T_BARRIER)
-		fdatasync(vblk->fd);
-
 	/* Finished that request. */
 	add_used(vq, head, wlen);
 }
@@ -1735,8 +1730,9 @@ static void setup_block_file(const char *filename)
 	vblk->fd = open_or_die(filename, O_RDWR|O_LARGEFILE);
 	vblk->len = lseek64(vblk->fd, 0, SEEK_END);
 
-	/* We support barriers. */
-	add_feature(dev, VIRTIO_BLK_F_BARRIER);
+	/* We support FLUSH and FUA. */
+	add_feature(dev, VIRTIO_BLK_F_FLUSH);
+	add_feature(dev, VIRTIO_BLK_F_FUA);
 
 	/* Tell Guest how many sectors this device has. */
 	conf.capacity = cpu_to_le64(vblk->len / 512);
Signature · 1 line
1.7.1
author
#16 lore raw

PATCH 4/5 md: implment REQ_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

This patch converts md to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER. In the core part (md.c), the following changes are notable.

* Unlike REQ_HARDBARRIER, REQ_FLUSH/FUA don't interfere with processing of other requests and thus there is no reason to mark the queue congested while FLUSH/FUA is in progress.

* REQ_FLUSH/FUA failures are final and its users don't need retry logic. Retry logic is removed.

* Preflush needs to be issued to all member devices but FUA writes can be handled the same way as other writes - their processing can be deferred to request_queue of member devices. md_barrier_request() is renamed to md_flush_request() and simplified accordingly.

For linear, raid0 and multipath, the core changes are enough. raid1, 5 and 10 need the following conversions.

* raid1: Handling of FLUSH/FUA bio's can simply be deferred to request_queues of member devices. Barrier related logic removed.

* raid5: Queue draining logic dropped. FUA bit is propagated through biodrain and stripe resconstruction such that all the updated parts of the stripe are written out with FUA writes if any of the dirtying writes was FUA. I think preread_active wait logic can be dropped but wasn't sure. If it can be dropped, please go ahead and drop it.

* raid10: FUA bit needs to be propagated to write clones.

linear, raid0, 1, 5 and 10 tested.

---
 drivers/md/linear.c    |    4 +-
 drivers/md/md.c        |  117 +++++++-------------------------
 drivers/md/md.h        |   23 ++-----
 drivers/md/multipath.c |    4 +-
 drivers/md/raid0.c     |    4 +-
 drivers/md/raid1.c     |  175 ++++++++++++++++--------------------------------
 drivers/md/raid1.h     |    2 -
 drivers/md/raid10.c    |    7 +-
 drivers/md/raid5.c     |   38 ++++++-----
 drivers/md/raid5.h     |    1 +
 10 files changed, 123 insertions(+), 252 deletions(-)
drivers/md/linear.c +2 -2
index ba19060..8a2f767 100644
@@ -294,8 +294,8 @@ static int linear_make_request (mddev_t *mddev, struct bio *bio)
 	dev_info_t *tmp_dev;
 	sector_t start_sector;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/md.c +25 -92

247 lines not on this page · raw message

drivers/md/md.h +6 -17
index fc56e0f..de4a365 100644
@@ -67,7 +67,6 @@ struct mdk_rdev_s
 #define	Faulty		1		/* device is known to have a fault */
 #define	In_sync		2		/* device is in_sync with rest of array */
 #define	WriteMostly	4		/* Avoid reading if at all possible */
-#define	BarriersNotsupp	5		/* REQ_HARDBARRIER is not supported */
 #define	AllReserved	6		/* If whole device is reserved for
 					 * one array */
 #define	AutoDetected	7		/* added by auto-detect */
@@ -249,13 +248,6 @@ struct mddev_s
 	int				degraded;	/* whether md should consider
 							 * adding a spare
 							 */
-	int				barriers_work;	/* initialised to true, cleared as soon
-							 * as a barrier request to slave
-							 * fails.  Only supported
-							 */
-	struct bio			*biolist; 	/* bios that need to be retried
-							 * because REQ_HARDBARRIER is not supported
-							 */
 
 	atomic_t			recovery_active; /* blocks scheduled, but not written */
 	wait_queue_head_t		recovery_wait;
@@ -308,16 +300,13 @@ struct mddev_s
 	struct list_head		all_mddevs;
 
 	struct attribute_group		*to_remove;
-	/* Generic barrier handling.
-	 * If there is a pending barrier request, all other
-	 * writes are blocked while the devices are flushed.
-	 * The last to finish a flush schedules a worker to
-	 * submit the barrier request (without the barrier flag),
-	 * then submit more flush requests.
+	/* Generic flush handling.
+	 * The last to finish preflush schedules a worker to submit
+	 * the rest of the request (without the REQ_FLUSH flag).
 	 */
-	struct bio *barrier;
+	struct bio *flush_bio;
 	atomic_t flush_pending;
-	struct work_struct barrier_work;
+	struct work_struct flush_work;
 };
 
 
@@ -458,7 +447,7 @@ extern void md_done_sync(mddev_t *mddev, int blocks, int ok);
 extern void md_error(mddev_t *mddev, mdk_rdev_t *rdev);
 
 extern int mddev_congested(mddev_t *mddev, int bits);
-extern void md_barrier_request(mddev_t *mddev, struct bio *bio);
+extern void md_flush_request(mddev_t *mddev, struct bio *bio);
 extern void md_super_write(mddev_t *mddev, mdk_rdev_t *rdev,
 			   sector_t sector, int size, struct page *page);
 extern void md_super_wait(mddev_t *mddev);
drivers/md/multipath.c +2 -2
index 0307d21..6d7ddf3 100644
@@ -142,8 +142,8 @@ static int multipath_make_request(mddev_t *mddev, struct bio * bio)
 	struct multipath_bh * mp_bh;
 	struct multipath_info *multipath;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/raid0.c +2 -2
index 6f7af46..a39f4c3 100644
@@ -483,8 +483,8 @@ static int raid0_make_request(mddev_t *mddev, struct bio *bio)
 	struct strip_zone *zone;
 	mdk_rdev_t *tmp_dev;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/raid1.c +59 -116

228 lines not on this page · raw message

drivers/md/raid1.h +0 -2
index 5f2d443..adf8cfd 100644
@@ -117,8 +117,6 @@ struct r1bio_s {
 #define	R1BIO_IsSync	1
 #define	R1BIO_Degraded	2
 #define	R1BIO_BehindIO	3
-#define	R1BIO_Barrier	4
-#define R1BIO_BarrierRetry 5
 /* For write-behind requests, we call bi_end_io when
  * the last non-write-behind device completes, providing
  * any write was successful.  Otherwise we call when
drivers/md/raid10.c +4 -3
index 5551ccf..cc00c10 100644
@@ -800,12 +800,13 @@ static int make_request(mddev_t *mddev, struct bio * bio)
 	int chunk_sects = conf->chunk_mask + 1;
 	const int rw = bio_data_dir(bio);
 	const unsigned int do_sync = (bio->bi_rw & REQ_SYNC);
+	const unsigned int do_fua = (bio->bi_rw & REQ_FUA);
 	struct bio_list bl;
 	unsigned long flags;
 	mdk_rdev_t *blocked_rdev;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
@@ -947,7 +948,7 @@ static int make_request(mddev_t *mddev, struct bio * bio)
 			conf->mirrors[d].rdev->data_offset;
 		mbio->bi_bdev = conf->mirrors[d].rdev->bdev;
 		mbio->bi_end_io	= raid10_end_write_request;
-		mbio->bi_rw = WRITE | do_sync;
+		mbio->bi_rw = WRITE | do_sync | do_fua;
 		mbio->bi_private = r10_bio;
 
 		atomic_inc(&r10_bio->remaining);
drivers/md/raid5.c +22 -16
index 20ac2f1..7eabd99 100644
@@ -507,9 +507,12 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 		int rw;
 		struct bio *bi;
 		mdk_rdev_t *rdev;
-		if (test_and_clear_bit(R5_Wantwrite, &sh->dev[i].flags))
-			rw = WRITE;
-		else if (test_and_clear_bit(R5_Wantread, &sh->dev[i].flags))
+		if (test_and_clear_bit(R5_Wantwrite, &sh->dev[i].flags)) {
+			if (test_and_clear_bit(R5_WantFUA, &sh->dev[i].flags))
+				rw = WRITE_FUA;
+			else
+				rw = WRITE;
+		} else if (test_and_clear_bit(R5_Wantread, &sh->dev[i].flags))
 			rw = READ;
 		else
 			continue;
@@ -1032,6 +1035,8 @@ ops_run_biodrain(struct stripe_head *sh, struct dma_async_tx_descriptor *tx)
 
 			while (wbi && wbi->bi_sector <
 				dev->sector + STRIPE_SECTORS) {
+				if (wbi->bi_rw & REQ_FUA)
+					set_bit(R5_WantFUA, &dev->flags);
 				tx = async_copy_data(1, wbi, dev->page,
 					dev->sector, tx);
 				wbi = r5_next_bio(wbi, dev->sector);
@@ -1049,15 +1054,22 @@ static void ops_complete_reconstruct(void *stripe_head_ref)
 	int pd_idx = sh->pd_idx;
 	int qd_idx = sh->qd_idx;
 	int i;
+	bool fua = false;
 
 	pr_debug("%s: stripe %llu\n", __func__,
 		(unsigned long long)sh->sector);
 
+	for (i = disks; i--; )
+		fua |= test_bit(R5_WantFUA, &sh->dev[i].flags);
+
 	for (i = disks; i--; ) {
 		struct r5dev *dev = &sh->dev[i];
 
-		if (dev->written || i == pd_idx || i == qd_idx)
+		if (dev->written || i == pd_idx || i == qd_idx) {
 			set_bit(R5_UPTODATE, &dev->flags);
+			if (fua)
+				set_bit(R5_WantFUA, &dev->flags);
+		}
 	}
 
 	if (sh->reconstruct_state == reconstruct_state_drain_run)
@@ -3278,7 +3290,7 @@ static void handle_stripe5(struct stripe_head *sh)
 
 	if (dec_preread_active) {
 		/* We delay this until after ops_run_io so that if make_request
-		 * is waiting on a barrier, it won't continue until the writes
+		 * is waiting on a flush, it won't continue until the writes
 		 * have actually been submitted.
 		 */
 		atomic_dec(&conf->preread_active_stripes);
@@ -3580,7 +3592,7 @@ static void handle_stripe6(struct stripe_head *sh)
 
 	if (dec_preread_active) {
 		/* We delay this until after ops_run_io so that if make_request
-		 * is waiting on a barrier, it won't continue until the writes
+		 * is waiting on a flush, it won't continue until the writes
 		 * have actually been submitted.
 		 */
 		atomic_dec(&conf->preread_active_stripes);
@@ -3958,14 +3970,8 @@ static int make_request(mddev_t *mddev, struct bio * bi)
 	const int rw = bio_data_dir(bi);
 	int remaining;
 
-	if (unlikely(bi->bi_rw & REQ_HARDBARRIER)) {
-		/* Drain all pending writes.  We only really need
-		 * to ensure they have been submitted, but this is
-		 * easier.
-		 */
-		mddev->pers->quiesce(mddev, 1);
-		mddev->pers->quiesce(mddev, 0);
-		md_barrier_request(mddev, bi);
+	if (unlikely(bi->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bi);
 		return 0;
 	}
 
@@ -4083,7 +4089,7 @@ static int make_request(mddev_t *mddev, struct bio * bi)
 			finish_wait(&conf->wait_for_overlap, &w);
 			set_bit(STRIPE_HANDLE, &sh->state);
 			clear_bit(STRIPE_DELAYED, &sh->state);
-			if (mddev->barrier && 
+			if (mddev->flush_bio &&
 			    !test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
 				atomic_inc(&conf->preread_active_stripes);
 			release_stripe(sh);
@@ -4106,7 +4112,7 @@ static int make_request(mddev_t *mddev, struct bio * bi)
 		bio_endio(bi, 0);
 	}
 
-	if (mddev->barrier) {
+	if (mddev->flush_bio) {
 		/* We need to wait for the stripes to all be handled.
 		 * So: wait for preread_active_stripes to drop to 0.
 		 */
drivers/md/raid5.h +1 -0
index 0f86f5e..ff9cad2 100644
@@ -275,6 +275,7 @@ struct r6_state {
 				    * filling
 				    */
 #define R5_Wantdrain	13 /* dev->towrite needs to be drained */
+#define R5_WantFUA	14	/* Write should be FUA */
 /*
  * Write method
  */
Signature · 1 line
1.7.1
#17 lore raw

Re: [PATCH 4/5] md: implment REQ_FLUSH/FUA support

On Mon, 16 Aug 2010 18:52:02 +0200
Tejun Heo <[email protected]> wrote:

Hi Tejun,
thanks for doing this.
It mostly looks good, especially ...

Quoted 2 lines

* REQ_FLUSH/FUA failures are final and its users don't need retry logic. Retry logic is removed.

This bit - all that retry logic felt so clumsy :-)

Only change I would make is:

Quoted 11 lines
drivers/md/raid5.c +1 -1
@@ -4083,7 +4089,7 @@ static int make_request(mddev_t *mddev, struct bio * bi)
 			finish_wait(&conf->wait_for_overlap, &w);
 			set_bit(STRIPE_HANDLE, &sh->state);
 			clear_bit(STRIPE_DELAYED, &sh->state);
-			if (mddev->barrier && 
+			if (mddev->flush_bio &&
 			    !test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
 				atomic_inc(&conf->preread_active_stripes);
 			release_stripe(sh);

drivers/md/raid5.c static int make_request(mddev_t *mddev, struct bio * bi) · line 4118

Quoted 8 lines
drivers/md/raid5.c +1 -1
@@ -4106,7 +4112,7 @@ static int make_request(mddev_t *mddev, struct bio * bi)
 		bio_endio(bi, 0);
 	}
 
-	if (mddev->barrier) {
+	if (mddev->flush_bio) {
 		/* We need to wait for the stripes to all be handled.
 		 * So: wait for preread_active_stripes to drop to 0.
 		 */

These two in raid5.c aren't quite right.
The first should be changed to test
bi->bi_rw & REQ_SYNC
rather than
mddev->flush_bio.
(Assuming the REQ_SYNC means "don't bother waiting for more requests that might combine with this one to make it all go faster" which I think it does.)

For the second we can just drop the whole if statement.
It was needed so that the all the writes would go done to the underlying devices so that the null-barrier which would subsequently be passed to all those devices would go *after* the writes for the barrier request. As there is no longer a post-flush, that code can go.

Thanks a lot, and sorry for the delay in reviewing it. NeilBrown

Quoted 12 lines
drivers/md/raid5.h +1 -0
index 0f86f5e..ff9cad2 100644
@@ -275,6 +275,7 @@ struct r6_state {
 				    * filling
 				    */
 #define R5_Wantdrain	13 /* dev->towrite needs to be drained */
+#define R5_WantFUA	14	/* Write should be FUA */
 /*
  * Write method
  */
author
#18 lore raw

PATCH 4/5 md: implment REQ_FLUSH/FUA support

This patch converts md to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER. In the core part (md.c), the following changes are notable.

* Unlike REQ_HARDBARRIER, REQ_FLUSH/FUA don't interfere with processing of other requests and thus there is no reason to mark the queue congested while FLUSH/FUA is in progress.

* REQ_FLUSH/FUA failures are final and its users don't need retry logic. Retry logic is removed.

* Preflush needs to be issued to all member devices but FUA writes can be handled the same way as other writes - their processing can be deferred to request_queue of member devices. md_barrier_request() is renamed to md_flush_request() and simplified accordingly.

For linear, raid0 and multipath, the core changes are enough. raid1, 5 and 10 need the following conversions.

* raid1: Handling of FLUSH/FUA bio's can simply be deferred to request_queues of member devices. Barrier related logic removed.

* raid5: Queue draining logic dropped. FUA bit is propagated through biodrain and stripe resconstruction such that all the updated parts of the stripe are written out with FUA writes if any of the dirtying writes was FUA. preread_active_stripes handling in make_request() is updated as suggested by Neil Brown.

* raid10: FUA bit needs to be propagated to write clones.

linear, raid0, 1, 5 and 10 tested.

---
Rebased on top of -rc2 and updated as suggested. Can you please review and ack it?

Thanks.

 drivers/md/linear.c    |    4 -
 drivers/md/md.c        |  117 +++++++--------------------------
 drivers/md/md.h        |   23 +-----
 drivers/md/multipath.c |    4 -
 drivers/md/raid0.c     |    4 -
 drivers/md/raid1.c     |  173 ++++++++++++++++---------------------------------
 drivers/md/raid1.h     |    2
 drivers/md/raid10.c    |    7 +
 drivers/md/raid5.c     |   43 +++++-------
 drivers/md/raid5.h     |    1
 10 files changed, 121 insertions(+), 257 deletions(-)
drivers/md/linear.c +2 -2
Index: block/drivers/md/linear.c
===================================================================
@@ -294,8 +294,8 @@ static int linear_make_request (mddev_t
 	dev_info_t *tmp_dev;
 	sector_t start_sector;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/md.c +25 -92

247 lines not on this page · raw message

drivers/md/md.h +6 -17
Index: block/drivers/md/md.h
===================================================================
@@ -87,7 +87,6 @@ struct mdk_rdev_s
 #define	Faulty		1		/* device is known to have a fault */
 #define	In_sync		2		/* device is in_sync with rest of array */
 #define	WriteMostly	4		/* Avoid reading if at all possible */
-#define	BarriersNotsupp	5		/* REQ_HARDBARRIER is not supported */
 #define	AllReserved	6		/* If whole device is reserved for
 					 * one array */
 #define	AutoDetected	7		/* added by auto-detect */
@@ -273,13 +272,6 @@ struct mddev_s
 	int				degraded;	/* whether md should consider
 							 * adding a spare
 							 */
-	int				barriers_work;	/* initialised to true, cleared as soon
-							 * as a barrier request to slave
-							 * fails.  Only supported
-							 */
-	struct bio			*biolist; 	/* bios that need to be retried
-							 * because REQ_HARDBARRIER is not supported
-							 */
 
 	atomic_t			recovery_active; /* blocks scheduled, but not written */
 	wait_queue_head_t		recovery_wait;
@@ -339,16 +331,13 @@ struct mddev_s
 	struct attribute_group		*to_remove;
 	struct plug_handle		*plug; /* if used by personality */
 
-	/* Generic barrier handling.
-	 * If there is a pending barrier request, all other
-	 * writes are blocked while the devices are flushed.
-	 * The last to finish a flush schedules a worker to
-	 * submit the barrier request (without the barrier flag),
-	 * then submit more flush requests.
+	/* Generic flush handling.
+	 * The last to finish preflush schedules a worker to submit
+	 * the rest of the request (without the REQ_FLUSH flag).
 	 */
-	struct bio *barrier;
+	struct bio *flush_bio;
 	atomic_t flush_pending;
-	struct work_struct barrier_work;
+	struct work_struct flush_work;
 	struct work_struct event_work;	/* used by dm to report failure event */
 };
 
@@ -502,7 +491,7 @@ extern void md_done_sync(mddev_t *mddev,
 extern void md_error(mddev_t *mddev, mdk_rdev_t *rdev);
 
 extern int mddev_congested(mddev_t *mddev, int bits);
-extern void md_barrier_request(mddev_t *mddev, struct bio *bio);
+extern void md_flush_request(mddev_t *mddev, struct bio *bio);
 extern void md_super_write(mddev_t *mddev, mdk_rdev_t *rdev,
 			   sector_t sector, int size, struct page *page);
 extern void md_super_wait(mddev_t *mddev);
drivers/md/raid0.c +2 -2
Index: block/drivers/md/raid0.c
===================================================================
@@ -483,8 +483,8 @@ static int raid0_make_request(mddev_t *m
 	struct strip_zone *zone;
 	mdk_rdev_t *tmp_dev;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/raid1.c +58 -115

227 lines not on this page · raw message

drivers/md/raid1.h +0 -2
Index: block/drivers/md/raid1.h
===================================================================
@@ -117,8 +117,6 @@ struct r1bio_s {
 #define	R1BIO_IsSync	1
 #define	R1BIO_Degraded	2
 #define	R1BIO_BehindIO	3
-#define	R1BIO_Barrier	4
-#define R1BIO_BarrierRetry 5
 /* For write-behind requests, we call bi_end_io when
  * the last non-write-behind device completes, providing
  * any write was successful.  Otherwise we call when
drivers/md/raid5.c +21 -22

99 lines not on this page · raw message

drivers/md/multipath.c +2 -2
Index: block/drivers/md/multipath.c
===================================================================
@@ -142,8 +142,8 @@ static int multipath_make_request(mddev_
 	struct multipath_bh * mp_bh;
 	struct multipath_info *multipath;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/raid10.c +4 -3
Index: block/drivers/md/raid10.c
===================================================================
@@ -800,12 +800,13 @@ static int make_request(mddev_t *mddev,
 	int chunk_sects = conf->chunk_mask + 1;
 	const int rw = bio_data_dir(bio);
 	const unsigned long do_sync = (bio->bi_rw & REQ_SYNC);
+	const unsigned long do_fua = (bio->bi_rw & REQ_FUA);
 	struct bio_list bl;
 	unsigned long flags;
 	mdk_rdev_t *blocked_rdev;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
@@ -965,7 +966,7 @@ static int make_request(mddev_t *mddev,
 			conf->mirrors[d].rdev->data_offset;
 		mbio->bi_bdev = conf->mirrors[d].rdev->bdev;
 		mbio->bi_end_io	= raid10_end_write_request;
-		mbio->bi_rw = WRITE | do_sync;
+		mbio->bi_rw = WRITE | do_sync | do_fua;
 		mbio->bi_private = r10_bio;
 
 		atomic_inc(&r10_bio->remaining);
drivers/md/raid5.h +1 -0
Index: block/drivers/md/raid5.h
===================================================================
@@ -275,6 +275,7 @@ struct r6_state {
 				    * filling
 				    */
 #define R5_Wantdrain	13 /* dev->towrite needs to be drained */
+#define R5_WantFUA	14	/* Write should be FUA */
 /*
  * Write method
  */
Reviewed-by on v1 4/5
#19 lore raw

Re: [PATCH UPDATED 4/5] md: implment REQ_FLUSH/FUA support

Quoted 30 lines · On Wed, 25 Aug 2010 13:22:26 +0200 Tejun Heo <[email protected]> wrote:

This patch converts md to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER. In the core part (md.c), the following changes are notable.

* Unlike REQ_HARDBARRIER, REQ_FLUSH/FUA don't interfere with processing of other requests and thus there is no reason to mark the queue congested while FLUSH/FUA is in progress.

* REQ_FLUSH/FUA failures are final and its users don't need retry logic. Retry logic is removed.

* Preflush needs to be issued to all member devices but FUA writes can be handled the same way as other writes - their processing can be deferred to request_queue of member devices. md_barrier_request() is renamed to md_flush_request() and simplified accordingly.

For linear, raid0 and multipath, the core changes are enough. raid1, 5 and 10 need the following conversions.

* raid1: Handling of FLUSH/FUA bio's can simply be deferred to request_queues of member devices. Barrier related logic removed.

* raid5: Queue draining logic dropped. FUA bit is propagated through biodrain and stripe resconstruction such that all the updated parts of the stripe are written out with FUA writes if any of the dirtying writes was FUA. preread_active_stripes handling in make_request() is updated as suggested by Neil Brown.

* raid10: FUA bit needs to be propagated to write clones.

Quoted 7 lines

linear, raid0, 1, 5 and 10 tested.

---
Rebased on top of -rc2 and updated as suggested. Can you please review and ack it?

Thanks!

NeilBrown

author
#20 lore raw

PATCH 4/5 md: implment REQ_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

This patch converts md to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER. In the core part (md.c), the following changes are notable.

* Unlike REQ_HARDBARRIER, REQ_FLUSH/FUA don't interfere with processing of other requests and thus there is no reason to mark the queue congested while FLUSH/FUA is in progress.

* REQ_FLUSH/FUA failures are final and its users don't need retry logic. Retry logic is removed.

* Preflush needs to be issued to all member devices but FUA writes can be handled the same way as other writes - their processing can be deferred to request_queue of member devices. md_barrier_request() is renamed to md_flush_request() and simplified accordingly.

For linear, raid0 and multipath, the core changes are enough. raid1, 5 and 10 need the following conversions.

* raid1: Handling of FLUSH/FUA bio's can simply be deferred to request_queues of member devices. Barrier related logic removed.

* raid5: Queue draining logic dropped. FUA bit is propagated through biodrain and stripe resconstruction such that all the updated parts of the stripe are written out with FUA writes if any of the dirtying writes was FUA. I think preread_active wait logic can be dropped but wasn't sure. If it can be dropped, please go ahead and drop it.

* raid10: FUA bit needs to be propagated to write clones.

linear, raid0, 1, 5 and 10 tested.

---
 drivers/md/linear.c    |    4 +-
 drivers/md/md.c        |  117 +++++++-------------------------
 drivers/md/md.h        |   23 ++-----
 drivers/md/multipath.c |    4 +-
 drivers/md/raid0.c     |    4 +-
 drivers/md/raid1.c     |  175 ++++++++++++++++--------------------------------
 drivers/md/raid1.h     |    2 -
 drivers/md/raid10.c    |    7 +-
 drivers/md/raid5.c     |   38 ++++++-----
 drivers/md/raid5.h     |    1 +
 10 files changed, 123 insertions(+), 252 deletions(-)
drivers/md/linear.c +2 -2
index ba19060..8a2f767 100644
@@ -294,8 +294,8 @@ static int linear_make_request (mddev_t *mddev, struct bio *bio)
 	dev_info_t *tmp_dev;
 	sector_t start_sector;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/md.c +25 -92

247 lines not on this page · raw message

drivers/md/md.h +6 -17
index fc56e0f..de4a365 100644
@@ -67,7 +67,6 @@ struct mdk_rdev_s
 #define	Faulty		1		/* device is known to have a fault */
 #define	In_sync		2		/* device is in_sync with rest of array */
 #define	WriteMostly	4		/* Avoid reading if at all possible */
-#define	BarriersNotsupp	5		/* REQ_HARDBARRIER is not supported */
 #define	AllReserved	6		/* If whole device is reserved for
 					 * one array */
 #define	AutoDetected	7		/* added by auto-detect */
@@ -249,13 +248,6 @@ struct mddev_s
 	int				degraded;	/* whether md should consider
 							 * adding a spare
 							 */
-	int				barriers_work;	/* initialised to true, cleared as soon
-							 * as a barrier request to slave
-							 * fails.  Only supported
-							 */
-	struct bio			*biolist; 	/* bios that need to be retried
-							 * because REQ_HARDBARRIER is not supported
-							 */
 
 	atomic_t			recovery_active; /* blocks scheduled, but not written */
 	wait_queue_head_t		recovery_wait;
@@ -308,16 +300,13 @@ struct mddev_s
 	struct list_head		all_mddevs;
 
 	struct attribute_group		*to_remove;
-	/* Generic barrier handling.
-	 * If there is a pending barrier request, all other
-	 * writes are blocked while the devices are flushed.
-	 * The last to finish a flush schedules a worker to
-	 * submit the barrier request (without the barrier flag),
-	 * then submit more flush requests.
+	/* Generic flush handling.
+	 * The last to finish preflush schedules a worker to submit
+	 * the rest of the request (without the REQ_FLUSH flag).
 	 */
-	struct bio *barrier;
+	struct bio *flush_bio;
 	atomic_t flush_pending;
-	struct work_struct barrier_work;
+	struct work_struct flush_work;
 };
 
 
@@ -458,7 +447,7 @@ extern void md_done_sync(mddev_t *mddev, int blocks, int ok);
 extern void md_error(mddev_t *mddev, mdk_rdev_t *rdev);
 
 extern int mddev_congested(mddev_t *mddev, int bits);
-extern void md_barrier_request(mddev_t *mddev, struct bio *bio);
+extern void md_flush_request(mddev_t *mddev, struct bio *bio);
 extern void md_super_write(mddev_t *mddev, mdk_rdev_t *rdev,
 			   sector_t sector, int size, struct page *page);
 extern void md_super_wait(mddev_t *mddev);
drivers/md/multipath.c +2 -2
index 0307d21..6d7ddf3 100644
@@ -142,8 +142,8 @@ static int multipath_make_request(mddev_t *mddev, struct bio * bio)
 	struct multipath_bh * mp_bh;
 	struct multipath_info *multipath;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/raid0.c +2 -2
index 6f7af46..a39f4c3 100644
@@ -483,8 +483,8 @@ static int raid0_make_request(mddev_t *mddev, struct bio *bio)
 	struct strip_zone *zone;
 	mdk_rdev_t *tmp_dev;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
drivers/md/raid1.c +59 -116

228 lines not on this page · raw message

drivers/md/raid1.h +0 -2
index 5f2d443..adf8cfd 100644
@@ -117,8 +117,6 @@ struct r1bio_s {
 #define	R1BIO_IsSync	1
 #define	R1BIO_Degraded	2
 #define	R1BIO_BehindIO	3
-#define	R1BIO_Barrier	4
-#define R1BIO_BarrierRetry 5
 /* For write-behind requests, we call bi_end_io when
  * the last non-write-behind device completes, providing
  * any write was successful.  Otherwise we call when
drivers/md/raid10.c +4 -3
index 5551ccf..cc00c10 100644
@@ -800,12 +800,13 @@ static int make_request(mddev_t *mddev, struct bio * bio)
 	int chunk_sects = conf->chunk_mask + 1;
 	const int rw = bio_data_dir(bio);
 	const unsigned int do_sync = (bio->bi_rw & REQ_SYNC);
+	const unsigned int do_fua = (bio->bi_rw & REQ_FUA);
 	struct bio_list bl;
 	unsigned long flags;
 	mdk_rdev_t *blocked_rdev;
 
-	if (unlikely(bio->bi_rw & REQ_HARDBARRIER)) {
-		md_barrier_request(mddev, bio);
+	if (unlikely(bio->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bio);
 		return 0;
 	}
 
@@ -947,7 +948,7 @@ static int make_request(mddev_t *mddev, struct bio * bio)
 			conf->mirrors[d].rdev->data_offset;
 		mbio->bi_bdev = conf->mirrors[d].rdev->bdev;
 		mbio->bi_end_io	= raid10_end_write_request;
-		mbio->bi_rw = WRITE | do_sync;
+		mbio->bi_rw = WRITE | do_sync | do_fua;
 		mbio->bi_private = r10_bio;
 
 		atomic_inc(&r10_bio->remaining);
drivers/md/raid5.c +22 -16
index 20ac2f1..7eabd99 100644
@@ -507,9 +507,12 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 		int rw;
 		struct bio *bi;
 		mdk_rdev_t *rdev;
-		if (test_and_clear_bit(R5_Wantwrite, &sh->dev[i].flags))
-			rw = WRITE;
-		else if (test_and_clear_bit(R5_Wantread, &sh->dev[i].flags))
+		if (test_and_clear_bit(R5_Wantwrite, &sh->dev[i].flags)) {
+			if (test_and_clear_bit(R5_WantFUA, &sh->dev[i].flags))
+				rw = WRITE_FUA;
+			else
+				rw = WRITE;
+		} else if (test_and_clear_bit(R5_Wantread, &sh->dev[i].flags))
 			rw = READ;
 		else
 			continue;
@@ -1032,6 +1035,8 @@ ops_run_biodrain(struct stripe_head *sh, struct dma_async_tx_descriptor *tx)
 
 			while (wbi && wbi->bi_sector <
 				dev->sector + STRIPE_SECTORS) {
+				if (wbi->bi_rw & REQ_FUA)
+					set_bit(R5_WantFUA, &dev->flags);
 				tx = async_copy_data(1, wbi, dev->page,
 					dev->sector, tx);
 				wbi = r5_next_bio(wbi, dev->sector);
@@ -1049,15 +1054,22 @@ static void ops_complete_reconstruct(void *stripe_head_ref)
 	int pd_idx = sh->pd_idx;
 	int qd_idx = sh->qd_idx;
 	int i;
+	bool fua = false;
 
 	pr_debug("%s: stripe %llu\n", __func__,
 		(unsigned long long)sh->sector);
 
+	for (i = disks; i--; )
+		fua |= test_bit(R5_WantFUA, &sh->dev[i].flags);
+
 	for (i = disks; i--; ) {
 		struct r5dev *dev = &sh->dev[i];
 
-		if (dev->written || i == pd_idx || i == qd_idx)
+		if (dev->written || i == pd_idx || i == qd_idx) {
 			set_bit(R5_UPTODATE, &dev->flags);
+			if (fua)
+				set_bit(R5_WantFUA, &dev->flags);
+		}
 	}
 
 	if (sh->reconstruct_state == reconstruct_state_drain_run)
@@ -3278,7 +3290,7 @@ static void handle_stripe5(struct stripe_head *sh)
 
 	if (dec_preread_active) {
 		/* We delay this until after ops_run_io so that if make_request
-		 * is waiting on a barrier, it won't continue until the writes
+		 * is waiting on a flush, it won't continue until the writes
 		 * have actually been submitted.
 		 */
 		atomic_dec(&conf->preread_active_stripes);
@@ -3580,7 +3592,7 @@ static void handle_stripe6(struct stripe_head *sh)
 
 	if (dec_preread_active) {
 		/* We delay this until after ops_run_io so that if make_request
-		 * is waiting on a barrier, it won't continue until the writes
+		 * is waiting on a flush, it won't continue until the writes
 		 * have actually been submitted.
 		 */
 		atomic_dec(&conf->preread_active_stripes);
@@ -3958,14 +3970,8 @@ static int make_request(mddev_t *mddev, struct bio * bi)
 	const int rw = bio_data_dir(bi);
 	int remaining;
 
-	if (unlikely(bi->bi_rw & REQ_HARDBARRIER)) {
-		/* Drain all pending writes.  We only really need
-		 * to ensure they have been submitted, but this is
-		 * easier.
-		 */
-		mddev->pers->quiesce(mddev, 1);
-		mddev->pers->quiesce(mddev, 0);
-		md_barrier_request(mddev, bi);
+	if (unlikely(bi->bi_rw & REQ_FLUSH)) {
+		md_flush_request(mddev, bi);
 		return 0;
 	}
 
@@ -4083,7 +4089,7 @@ static int make_request(mddev_t *mddev, struct bio * bi)
 			finish_wait(&conf->wait_for_overlap, &w);
 			set_bit(STRIPE_HANDLE, &sh->state);
 			clear_bit(STRIPE_DELAYED, &sh->state);
-			if (mddev->barrier && 
+			if (mddev->flush_bio &&
 			    !test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
 				atomic_inc(&conf->preread_active_stripes);
 			release_stripe(sh);
@@ -4106,7 +4112,7 @@ static int make_request(mddev_t *mddev, struct bio * bi)
 		bio_endio(bi, 0);
 	}
 
-	if (mddev->barrier) {
+	if (mddev->flush_bio) {
 		/* We need to wait for the stripes to all be handled.
 		 * So: wait for preread_active_stripes to drop to 0.
 		 */
drivers/md/raid5.h +1 -0
index 0f86f5e..ff9cad2 100644
@@ -275,6 +275,7 @@ struct r6_state {
 				    * filling
 				    */
 #define R5_Wantdrain	13 /* dev->towrite needs to be drained */
+#define R5_WantFUA	14	/* Write should be FUA */
 /*
  * Write method
  */
Signature · 1 line
1.7.1
author
#21 lore raw

PATCH 5/5 dm: implement REQ_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

This patch converts dm to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER.

For common parts,

* Barrier retry logic dropped from dm-io.

* Empty WRITE_BARRIERs are replaced with WRITE_FLUSHes.

* It's now guaranteed that all FLUSH bio's which are passed onto dm targets are zero length. bio_empty_barrier() tests are replaced with REQ_FLUSH tests.

* Dropped unlikely() around REQ_FLUSH tests. Flushes are not unlikely enough to be marked with unlikely().

For bio-based dm,

* Preflush is handled as before but postflush is dropped and replaced with passing down REQ_FUA to member request_queues. This replaces one array wide cache flush w/ member specific FUA writes.

* __split_and_process_bio() now calls __clone_and_map_flush() directly for flushes and guarantees all FLUSH bio's going to targets are zero length.

* -EOPNOTSUPP retry logic dropped.

For request-based dm,

* Nothing much changes. It just needs to handle FLUSH requests as before. It would be beneficial to advertise FUA capability so that it can propagate FUA flags down to member request_queues instead of sequencing it as WRITE + FLUSH at the top queue.

Lightly tested linear, stripe, raid1, snap and crypt targets. Please proceed with caution as I'm not familiar with the code base.

---
 drivers/md/dm-crypt.c           |    2 +-
 drivers/md/dm-io.c              |   20 +----
 drivers/md/dm-log.c             |    2 +-
 drivers/md/dm-raid1.c           |    8 +-
 drivers/md/dm-region-hash.c     |   16 ++--
 drivers/md/dm-snap-persistent.c |    2 +-
 drivers/md/dm-snap.c            |    6 +-
 drivers/md/dm-stripe.c          |    2 +-
 drivers/md/dm.c                 |  180 +++++++++++++++++++-------------------
 9 files changed, 113 insertions(+), 125 deletions(-)
drivers/md/dm-crypt.c +1 -1
index 3bdbb61..da11652 100644
@@ -1249,7 +1249,7 @@ static int crypt_map(struct dm_target *ti, struct bio *bio,
 	struct dm_crypt_io *io;
 	struct crypt_config *cc;
 
-	if (unlikely(bio_empty_barrier(bio))) {
+	if (bio->bi_rw & REQ_FLUSH) {
 		cc = ti->private;
 		bio->bi_bdev = cc->dev->bdev;
 		return DM_MAPIO_REMAPPED;
drivers/md/dm-io.c +4 -16
index 0590c75..136d4f7 100644
@@ -31,7 +31,6 @@ struct dm_io_client {
  */
 struct io {
 	unsigned long error_bits;
-	unsigned long eopnotsupp_bits;
 	atomic_t count;
 	struct task_struct *sleeper;
 	struct dm_io_client *client;
@@ -130,11 +129,8 @@ static void retrieve_io_and_region_from_bio(struct bio *bio, struct io **io,
  *---------------------------------------------------------------*/
 static void dec_count(struct io *io, unsigned int region, int error)
 {
-	if (error) {
+	if (error)
 		set_bit(region, &io->error_bits);
-		if (error == -EOPNOTSUPP)
-			set_bit(region, &io->eopnotsupp_bits);
-	}
 
 	if (atomic_dec_and_test(&io->count)) {
 		if (io->sleeper)
@@ -310,8 +306,8 @@ static void do_region(int rw, unsigned region, struct dm_io_region *where,
 	sector_t remaining = where->count;
 
 	/*
-	 * where->count may be zero if rw holds a write barrier and we
-	 * need to send a zero-sized barrier.
+	 * where->count may be zero if rw holds a flush and we need to
+	 * send a zero-sized flush.
 	 */
 	do {
 		/*
@@ -364,7 +360,7 @@ static void dispatch_io(int rw, unsigned int num_regions,
 	 */
 	for (i = 0; i < num_regions; i++) {
 		*dp = old_pages;
-		if (where[i].count || (rw & REQ_HARDBARRIER))
+		if (where[i].count || (rw & REQ_FLUSH))
 			do_region(rw, i, where + i, dp, io);
 	}
 
@@ -393,9 +389,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
 		return -EIO;
 	}
 
-retry:
 	io->error_bits = 0;
-	io->eopnotsupp_bits = 0;
 	atomic_set(&io->count, 1); /* see dispatch_io() */
 	io->sleeper = current;
 	io->client = client;
@@ -412,11 +406,6 @@ retry:
 	}
 	set_current_state(TASK_RUNNING);
 
-	if (io->eopnotsupp_bits && (rw & REQ_HARDBARRIER)) {
-		rw &= ~REQ_HARDBARRIER;
-		goto retry;
-	}
-
 	if (error_bits)
 		*error_bits = io->error_bits;
 
@@ -437,7 +426,6 @@ static int async_io(struct dm_io_client *client, unsigned int num_regions,
 
 	io = mempool_alloc(client->pool, GFP_NOIO);
 	io->error_bits = 0;
-	io->eopnotsupp_bits = 0;
 	atomic_set(&io->count, 1); /* see dispatch_io() */
 	io->sleeper = NULL;
 	io->client = client;
drivers/md/dm-log.c +1 -1
index 5a08be0..33420e6 100644
@@ -300,7 +300,7 @@ static int flush_header(struct log_c *lc)
 		.count = 0,
 	};
 
-	lc->io_req.bi_rw = WRITE_BARRIER;
+	lc->io_req.bi_rw = WRITE_FLUSH;
 
 	return dm_io(&lc->io_req, 1, &null_location, NULL);
 }
drivers/md/dm-raid1.c +4 -4
index 7413626..af33245 100644
@@ -259,7 +259,7 @@ static int mirror_flush(struct dm_target *ti)
 	struct dm_io_region io[ms->nr_mirrors];
 	struct mirror *m;
 	struct dm_io_request io_req = {
-		.bi_rw = WRITE_BARRIER,
+		.bi_rw = WRITE_FLUSH,
 		.mem.type = DM_IO_KMEM,
 		.mem.ptr.bvec = NULL,
 		.client = ms->io_client,
@@ -629,7 +629,7 @@ static void do_write(struct mirror_set *ms, struct bio *bio)
 	struct dm_io_region io[ms->nr_mirrors], *dest = io;
 	struct mirror *m;
 	struct dm_io_request io_req = {
-		.bi_rw = WRITE | (bio->bi_rw & WRITE_BARRIER),
+		.bi_rw = WRITE | (bio->bi_rw & WRITE_FLUSH_FUA),
 		.mem.type = DM_IO_BVEC,
 		.mem.ptr.bvec = bio->bi_io_vec + bio->bi_idx,
 		.notify.fn = write_callback,
@@ -670,7 +670,7 @@ static void do_writes(struct mirror_set *ms, struct bio_list *writes)
 	bio_list_init(&requeue);
 
 	while ((bio = bio_list_pop(writes))) {
-		if (unlikely(bio_empty_barrier(bio))) {
+		if (bio->bi_rw & REQ_FLUSH) {
 			bio_list_add(&sync, bio);
 			continue;
 		}
@@ -1203,7 +1203,7 @@ static int mirror_end_io(struct dm_target *ti, struct bio *bio,
 	 * We need to dec pending if this was a write.
 	 */
 	if (rw == WRITE) {
-		if (likely(!bio_empty_barrier(bio)))
+		if (!(bio->bi_rw & REQ_FLUSH))
 			dm_rh_dec(ms->rh, map_context->ll);
 		return error;
 	}
drivers/md/dm-region-hash.c +8 -8
index bd5c58b..dad011a 100644
@@ -81,9 +81,9 @@ struct dm_region_hash {
 	struct list_head failed_recovered_regions;
 
 	/*
-	 * If there was a barrier failure no regions can be marked clean.
+	 * If there was a flush failure no regions can be marked clean.
 	 */
-	int barrier_failure;
+	int flush_failure;
 
 	void *context;
 	sector_t target_begin;
@@ -217,7 +217,7 @@ struct dm_region_hash *dm_region_hash_create(
 	INIT_LIST_HEAD(&rh->quiesced_regions);
 	INIT_LIST_HEAD(&rh->recovered_regions);
 	INIT_LIST_HEAD(&rh->failed_recovered_regions);
-	rh->barrier_failure = 0;
+	rh->flush_failure = 0;
 
 	rh->region_pool = mempool_create_kmalloc_pool(MIN_REGIONS,
 						      sizeof(struct dm_region));
@@ -399,8 +399,8 @@ void dm_rh_mark_nosync(struct dm_region_hash *rh, struct bio *bio)
 	region_t region = dm_rh_bio_to_region(rh, bio);
 	int recovering = 0;
 
-	if (bio_empty_barrier(bio)) {
-		rh->barrier_failure = 1;
+	if (bio->bi_rw & REQ_FLUSH) {
+		rh->flush_failure = 1;
 		return;
 	}
 
@@ -524,7 +524,7 @@ void dm_rh_inc_pending(struct dm_region_hash *rh, struct bio_list *bios)
 	struct bio *bio;
 
 	for (bio = bios->head; bio; bio = bio->bi_next) {
-		if (bio_empty_barrier(bio))
+		if (bio->bi_rw & REQ_FLUSH)
 			continue;
 		rh_inc(rh, dm_rh_bio_to_region(rh, bio));
 	}
@@ -555,9 +555,9 @@ void dm_rh_dec(struct dm_region_hash *rh, region_t region)
 		 */
 
 		/* do nothing for DM_RH_NOSYNC */
-		if (unlikely(rh->barrier_failure)) {
+		if (unlikely(rh->flush_failure)) {
 			/*
-			 * If a write barrier failed some time ago, we
+			 * If a write flush failed some time ago, we
 			 * don't know whether or not this write made it
 			 * to the disk, so we must resync the device.
 			 */
drivers/md/dm-snap-persistent.c +1 -1
index c097d8a..b9048f0 100644
@@ -687,7 +687,7 @@ static void persistent_commit_exception(struct dm_exception_store *store,
 	/*
 	 * Commit exceptions to disk.
 	 */
-	if (ps->valid && area_io(ps, WRITE_BARRIER))
+	if (ps->valid && area_io(ps, WRITE_FLUSH_FUA))
 		ps->valid = 0;
 
 	/*
drivers/md/dm-snap.c +3 -3
index 5485377..62a6586 100644
@@ -1581,7 +1581,7 @@ static int snapshot_map(struct dm_target *ti, struct bio *bio,
 	chunk_t chunk;
 	struct dm_snap_pending_exception *pe = NULL;
 
-	if (unlikely(bio_empty_barrier(bio))) {
+	if (bio->bi_rw & REQ_FLUSH) {
 		bio->bi_bdev = s->cow->bdev;
 		return DM_MAPIO_REMAPPED;
 	}
@@ -1685,7 +1685,7 @@ static int snapshot_merge_map(struct dm_target *ti, struct bio *bio,
 	int r = DM_MAPIO_REMAPPED;
 	chunk_t chunk;
 
-	if (unlikely(bio_empty_barrier(bio))) {
+	if (bio->bi_rw & REQ_FLUSH) {
 		if (!map_context->flush_request)
 			bio->bi_bdev = s->origin->bdev;
 		else
@@ -2123,7 +2123,7 @@ static int origin_map(struct dm_target *ti, struct bio *bio,
 	struct dm_dev *dev = ti->private;
 	bio->bi_bdev = dev->bdev;
 
-	if (unlikely(bio_empty_barrier(bio)))
+	if (bio->bi_rw & REQ_FLUSH)
 		return DM_MAPIO_REMAPPED;
 
 	/* Only tell snapshots if this is a write */
drivers/md/dm-stripe.c +1 -1
index d6e28d7..0f534ca 100644
@@ -214,7 +214,7 @@ static int stripe_map(struct dm_target *ti, struct bio *bio,
 	sector_t offset, chunk;
 	uint32_t stripe;
 
-	if (unlikely(bio_empty_barrier(bio))) {
+	if (bio->bi_rw & REQ_FLUSH) {
 		BUG_ON(map_context->flush_request >= sc->stripes);
 		bio->bi_bdev = sc->stripe[map_context->flush_request].dev->bdev;
 		return DM_MAPIO_REMAPPED;
drivers/md/dm.c +90 -90

425 lines not on this page · raw message

Signature · 1 line
1.7.1
#22 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

Quoted 4 lines · Tejun Heo

From: Tejun Heo <[email protected]>

This patch converts dm to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER.

What tree does this patch apply to? I know it doesn't apply to v2.6.36-rc1, e.g.: http://git.kernel.org/linus/708e929513502fb0

Quoted 1 line

For bio-based dm,

...

Quoted 1 line

* -EOPNOTSUPP retry logic dropped.

That logic wasn't just about retries (at least not in the latest kernel). With commit 708e929513502fb0 the -EOPNOTSUPP checking also serves to optimize the barrier+discard case (when discards aren't supported).

Quoted 6 lines

For request-based dm,

* Nothing much changes. It just needs to handle FLUSH requests as before. It would be beneficial to advertise FUA capability so that it can propagate FUA flags down to member request_queues instead of sequencing it as WRITE + FLUSH at the top queue.

Can you expand on that TODO a bit? What is the mechanism to propagate FUA down to a DM device's members? I'm only aware of propagating member devices' features up to the top-level DM device's request-queue (not the opposite).

Are you saying that establishing the FUA capability on the top-level DM device's request_queue is sufficient? If so then why not make the change?

Quoted 2 lines

Lightly tested linear, stripe, raid1, snap and crypt targets. Please proceed with caution as I'm not familiar with the code base.

This is concerning... if we're to offer more comprehensive review I think we need more detail on what guided your changes rather than details of what the resulting changes are.

Mike

author
#23 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

Hello,

Quoted 10 lines · Mike Snitzer
Quoted 4 lines · Tejun Heo

From: Tejun Heo <[email protected]>

This patch converts dm to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER.

What tree does this patch apply to? I know it doesn't apply to v2.6.36-rc1, e.g.: http://git.kernel.org/linus/708e929513502fb0

(from the head message)
These patches are on top of

block#for-2.6.36-post (c047ab2dddeeafbd6f7c00e45a13a5c4da53ea0b)
+ block-replace-barrier-with-sequenced-flush patchset[1]
+ block-fix-incorrect-bio-request-flag-conversion-in-md patch[2]

and available in the following git tree.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git flush-fua

[1] http://thread.gmane.org/gmane.linux.kernel/1022363
[2] http://thread.gmane.org/gmane.linux.kernel/1023435

Probably fetching the git tree is the easist way to review?

Quoted 7 lines
Quoted 2 lines

For bio-based dm,
* -EOPNOTSUPP retry logic dropped.

That logic wasn't just about retries (at least not in the latest kernel). With commit 708e929513502fb0 the -EOPNOTSUPP checking also serves to optimize the barrier+discard case (when discards aren't supported).

With the patch applied, there's no second flush. Those requests would now be REQ_FLUSH + REQ_DISCARD. The first can't be avoided anyway and there won't be the second flush to begin with, so I don't think this worsens anything.

Quoted 5 lines
Quoted 4 lines

* Nothing much changes. It just needs to handle FLUSH requests as before. It would be beneficial to advertise FUA capability so that it can propagate FUA flags down to member request_queues instead of sequencing it as WRITE + FLUSH at the top queue.

Quoted 8 lines

Can you expand on that TODO a bit? What is the mechanism to propagate FUA down to a DM device's members? I'm only aware of propagating member devices' features up to the top-level DM device's request-queue (not the opposite).

Are you saying that establishing the FUA capability on the top-level DM device's request_queue is sufficient? If so then why not make the change?

Yeah, I think it would be enough to always advertise FLUSH|FUA if the member devices support FLUSH (regardless of FUA support). The reason why I didn't do it was, umm, laziness, I suppose.

Quoted 4 lines
Quoted 2 lines

Lightly tested linear, stripe, raid1, snap and crypt targets. Please proceed with caution as I'm not familiar with the code base.

This is concerning...

Yeap, I want you to be concerned. :-) This was the first time I looked at the dm code and there are many different disjoint code paths and I couldn't fully follow or test all of them, so it definitely needs a careful review from someone who understands the whole thing.

Quoted 3 lines

if we're to offer more comprehensive review I think we need more detail on what guided your changes rather than details of what the resulting changes are.

I'll try to explain it. If you have any further questions, please let me know.

* For common part (dm-io, dm-log):

* Empty REQ_HARDBARRIER is converted to empty REQ_FLUSH.

* REQ_HARDBARRIER w/ data is converted either to data + REQ_FLUSH + REQ_FUA or data + REQ_FUA. The former is the safe equivalent conversion but there could be cases where ther latter is enough.

* For bio based dm:

* Unlike REQ_HARDBARRIER, REQ_FLUSH/FUA doesn't have any ordering requirements. Remove assumptions of ordering and/or draining.

A related question: Is dm_wait_for_completion() used in process_flush() safe against starvation under continuous influx of other commands?

* As REQ_FLUSH/FUA doesn't require any ordering of requests before or after it, on array devices, the latter part - REQ_FUA - can be handled like other writes. ie. REQ_FLUSH needs to be broadcasted to all devices but once that is complete the data/REQ_FUA bio can be sent to only the affected devices. This needs some care as there are bio cloning/splitting code paths where REQ_FUA bit isn't preserved.

* Guarantee that REQ_FLUSH w/ data never reaches targets (this in part is to put it in alignment with request based dm).

* For request based dm:

* The sequencing is done by the block layer for the top level request_queue, so the only things request based dm needs to make sure is 1. handling empty REQ_FLUSH correctly (block layer will only send down empty REQ_FLUSHes) and 2. propagate REQ_FUA bit to member devices.

Thanks.

Signature · 1 line
tejun
#24 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

Quoted 9 lines · Tejun Heo
Quoted 4 lines

That logic wasn't just about retries (at least not in the latest kernel). With commit 708e929513502fb0 the -EOPNOTSUPP checking also serves to optimize the barrier+discard case (when discards aren't supported).

With the patch applied, there's no second flush. Those requests would now be REQ_FLUSH + REQ_DISCARD. The first can't be avoided anyway and there won't be the second flush to begin with, so I don't think this worsens anything.

In fact the pre-flush is completely superflous for discards, but that's a separate discussion and should not be changed as part of this patchset but rather explicitly.

#25 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

In reply to Tejun Heo
Quoted 21 lines · Tejun Heo

Hello,

Quoted 10 lines · Mike Snitzer
Quoted 4 lines · Tejun Heo

From: Tejun Heo <[email protected]>

This patch converts dm to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER.

What tree does this patch apply to? I know it doesn't apply to v2.6.36-rc1, e.g.: http://git.kernel.org/linus/708e929513502fb0

(from the head message)
These patches are on top of

block#for-2.6.36-post (c047ab2dddeeafbd6f7c00e45a13a5c4da53ea0b)
+ block-replace-barrier-with-sequenced-flush patchset[1]
+ block-fix-incorrect-bio-request-flag-conversion-in-md patch[2]

Quoted 8 lines

and available in the following git tree.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git flush-fua

[1] http://thread.gmane.org/gmane.linux.kernel/1022363
[2] http://thread.gmane.org/gmane.linux.kernel/1023435

Probably fetching the git tree is the easist way to review?

OK, I missed this info because I just looked at the DM patch.

Quoted 8 lines
Quoted 7 lines
Quoted 2 lines

For bio-based dm,
* -EOPNOTSUPP retry logic dropped.

That logic wasn't just about retries (at least not in the latest kernel). With commit 708e929513502fb0 the -EOPNOTSUPP checking also serves to optimize the barrier+discard case (when discards aren't supported).

Quoted 4 lines

With the patch applied, there's no second flush. Those requests would now be REQ_FLUSH + REQ_DISCARD. The first can't be avoided anyway and there won't be the second flush to begin with, so I don't think this worsens anything.

Makes sense, but your patches still need to be refreshed against the latest (2.6.36-rc1) upstream code. Numerous changes went in to DM recently.

Quoted 10 lines
Quoted 10 lines
Quoted 4 lines

* Nothing much changes. It just needs to handle FLUSH requests as before. It would be beneficial to advertise FUA capability so that it can propagate FUA flags down to member request_queues instead of sequencing it as WRITE + FLUSH at the top queue.

Can you expand on that TODO a bit? What is the mechanism to propagate FUA down to a DM device's members? I'm only aware of propagating member devices' features up to the top-level DM device's request-queue (not the opposite).

Quoted 7 lines
Quoted 3 lines

Are you saying that establishing the FUA capability on the top-level DM device's request_queue is sufficient? If so then why not make the change?

Yeah, I think it would be enough to always advertise FLUSH|FUA if the member devices support FLUSH (regardless of FUA support). The reason why I didn't do it was, umm, laziness, I suppose.

I don't buy it.. you're far from lazy! ;)

Quoted 9 lines
Quoted 4 lines
Quoted 2 lines

Lightly tested linear, stripe, raid1, snap and crypt targets. Please proceed with caution as I'm not familiar with the code base.

This is concerning...

Yeap, I want you to be concerned. :-) This was the first time I looked at the dm code and there are many different disjoint code paths and I couldn't fully follow or test all of them, so it definitely needs a careful review from someone who understands the whole thing.

You'll need Mikulas (bio-based) and NEC (request-based, Kiyoshi and Jun'ichi) to give it serious review.

NOTE: NEC has already given some preliminary feedback to hch in the "[PATCH, RFC 2/2] dm: support REQ_FLUSH directly" thread:
https://www.redhat.com/archives/dm-devel/2010-August/msg00026.html
https://www.redhat.com/archives/dm-devel/2010-August/msg00033.html

Quoted 6 lines
Quoted 3 lines

if we're to offer more comprehensive review I think we need more detail on what guided your changes rather than details of what the resulting changes are.

I'll try to explain it. If you have any further questions, please let me know.

Thanks for the additional details.

Quoted 8 lines

* For bio based dm:

* Unlike REQ_HARDBARRIER, REQ_FLUSH/FUA doesn't have any ordering requirements. Remove assumptions of ordering and/or draining.

A related question: Is dm_wait_for_completion() used in process_flush() safe against starvation under continuous influx of other commands?

OK, so you folded dm_flush() directly into process_flush() -- the code that was dm_flush() only needs to be called once now.

As for your specific dm_wait_for_completion() concern -- I'll defer to Mikulas. But I'll add: we haven't had any reported starvation issues with DM's existing barrier support. DM uses a mempool for its clones, so it should naturally throttle (without starvation) when memory gets low.

Quoted 10 lines

* As REQ_FLUSH/FUA doesn't require any ordering of requests before or after it, on array devices, the latter part - REQ_FUA - can be handled like other writes. ie. REQ_FLUSH needs to be broadcasted to all devices but once that is complete the data/REQ_FUA bio can be sent to only the affected devices. This needs some care as there are bio cloning/splitting code paths where REQ_FUA bit isn't preserved.

* Guarantee that REQ_FLUSH w/ data never reaches targets (this in part is to put it in alignment with request based dm).

bio-based DM already split the barrier out from the data (in process_barrier). You've renamed process_barrier to process_flush and added the REQ_FLUSH logic like I'd expect.

Quoted 7 lines

* For request based dm:

* The sequencing is done by the block layer for the top level request_queue, so the only things request based dm needs to make sure is 1. handling empty REQ_FLUSH correctly (block layer will only send down empty REQ_FLUSHes) and 2. propagate REQ_FUA bit to member devices.

OK, so seems 1 is done, 2 is still TODO. Looking at your tree it seems 2 would be as simple as using the following in dm_init_request_based_queue (on the most current upstream dm.c):

blk_queue_flush(q, REQ_FLUSH | REQ_FUA);

(your current patch only sets REQ_FLUSH in alloc_dev).

author
#26 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

Hello,

Quoted 8 lines · Mike Snitzer
Quoted 4 lines

With the patch applied, there's no second flush. Those requests would now be REQ_FLUSH + REQ_DISCARD. The first can't be avoided anyway and there won't be the second flush to begin with, so I don't think this worsens anything.

Makes sense, but your patches still need to be refreshed against the latest (2.6.36-rc1) upstream code. Numerous changes went in to DM recently.

Sure thing. The block part isn't fixed yet and so the RFC tag. Once the block layer part is settled, it probably should be pulled into dm/md and other trees and conversions should happen there.

Quoted 7 lines
Quoted 4 lines

Yeap, I want you to be concerned. :-) This was the first time I looked at the dm code and there are many different disjoint code paths and I couldn't fully follow or test all of them, so it definitely needs a careful review from someone who understands the whole thing.

You'll need Mikulas (bio-based) and NEC (request-based, Kiyoshi and Jun'ichi) to give it serious review.

Oh, you already cc'd them. Great. Hello, guys, the original thread is

http://thread.gmane.org/gmane.linux.raid/29100

Quoted 4 lines

NOTE: NEC has already given some preliminary feedback to hch in the "[PATCH, RFC 2/2] dm: support REQ_FLUSH directly" thread:
https://www.redhat.com/archives/dm-devel/2010-August/msg00026.html
https://www.redhat.com/archives/dm-devel/2010-August/msg00033.html

Hmmm... I think both issues don't exist in this incarnation of conversion although I'm fairly sure there will be other issues. :-)

Quoted 9 lines
Quoted 3 lines

A related question: Is dm_wait_for_completion() used in process_flush() safe against starvation under continuous influx of other commands?

As for your specific dm_wait_for_completion() concern -- I'll defer to Mikulas. But I'll add: we haven't had any reported starvation issues with DM's existing barrier support. DM uses a mempool for its clones, so it should naturally throttle (without starvation) when memory gets low.

I see but single pending flush and steady write streams w/o saturating the mempool would be able to stall dm_wait_for_completeion(), no? Eh well, it's a separate issue, I guess.

Quoted 6 lines
Quoted 2 lines

* Guarantee that REQ_FLUSH w/ data never reaches targets (this in part is to put it in alignment with request based dm).

bio-based DM already split the barrier out from the data (in process_barrier). You've renamed process_barrier to process_flush and added the REQ_FLUSH logic like I'd expect.

Yeah and threw in WARN_ON() there to make sure REQ_FLUSH + data bios don't slip through for whatever reason.

Quoted 10 lines
Quoted 7 lines

* For request based dm:

* The sequencing is done by the block layer for the top level request_queue, so the only things request based dm needs to make sure is 1. handling empty REQ_FLUSH correctly (block layer will only send down empty REQ_FLUSHes) and 2. propagate REQ_FUA bit to member devices.

OK, so seems 1 is done, 2 is still TODO. Looking at your tree it seems 2 would be as simple as using the following in

Oh, I was talking about the other way around. Passing REQ_FUA in bio->bi_rw down to member request_queues. Sometimes while constructing clone / split bios, the bit is lost (e.g. md raid5).

Quoted 3 lines

dm_init_request_based_queue (on the most current upstream dm.c):
blk_queue_flush(q, REQ_FLUSH | REQ_FUA);
(your current patch only sets REQ_FLUSH in alloc_dev).

Yeah, but for that direction, just adding REQ_FUA to blk_queue_flush() should be enough. I'll add it.

Thanks.

Signature · 1 line
tejun
#27 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

Quoted 8 lines · Tejun Heo

Hello,

Quoted 5 lines · Mike Snitzer
Quoted 4 lines

With the patch applied, there's no second flush. Those requests would now be REQ_FLUSH + REQ_DISCARD. The first can't be avoided anyway and there won't be the second flush to begin with, so I don't think this worsens anything.

Quoted 7 lines
Quoted 3 lines

Makes sense, but your patches still need to be refreshed against the latest (2.6.36-rc1) upstream code. Numerous changes went in to DM recently.

Sure thing. The block part isn't fixed yet and so the RFC tag. Once the block layer part is settled, it probably should be pulled into dm/md and other trees and conversions should happen there.

Why base your work on a partial 2.6.36 tree? Why not rebase to linus' 2.6.36-rc1?

Once we get the changes in a more suitable state (across the entire tree) we can split the individual changes out to their respective trees. Without a comprehensive tree I fear this code isn't going to get tested or reviewed properly.

For example: any review of DM changes, against stale DM code, is wasted effort.

Quoted 8 lines
Quoted 8 lines
Quoted 7 lines

* For request based dm:

* The sequencing is done by the block layer for the top level request_queue, so the only things request based dm needs to make sure is 1. handling empty REQ_FLUSH correctly (block layer will only send down empty REQ_FLUSHes) and 2. propagate REQ_FUA bit to member devices.

Quoted 6 lines
Quoted 2 lines

OK, so seems 1 is done, 2 is still TODO. Looking at your tree it seems 2 would be as simple as using the following in

Oh, I was talking about the other way around. Passing REQ_FUA in bio->bi_rw down to member request_queues. Sometimes while constructing clone / split bios, the bit is lost (e.g. md raid5).

Seems we need to change __blk_rq_prep_clone to propagate REQ_FUA just like REQ_DISCARD: http://git.kernel.org/linus/3383977

Doesn't seem like we need to do the same for REQ_FLUSH (at least not for rq-based DM's benefit) because dm.c:setup_clone already special cases flush requests and sets REQ_FLUSH in cmd_flags.

Mike

author
#28 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

Hello,

Quoted 2 lines · Mike Snitzer

Why base your work on a partial 2.6.36 tree? Why not rebase to linus' 2.6.36-rc1?

Because the block tree contains enough changes which are not in mainline yet and bulk of the changes should go through the block tree.

Quoted 7 lines

Once we get the changes in a more suitable state (across the entire tree) we can split the individual changes out to their respective trees. Without a comprehensive tree I fear this code isn't going to get tested or reviewed properly.

For example: any review of DM changes, against stale DM code, is wasted effort.

Yeap, sure, it will happen all in a good time, but I don't really agree that review against block tree would be complete waste of effort. Things usually don't change that drastically unless dm is being rewritten as we speak. Anyways, I'll soon post a rebased / updated patch.

Quoted 6 lines
Quoted 3 lines

Oh, I was talking about the other way around. Passing REQ_FUA in bio->bi_rw down to member request_queues. Sometimes while constructing clone / split bios, the bit is lost (e.g. md raid5).

Seems we need to change __blk_rq_prep_clone to propagate REQ_FUA just like REQ_DISCARD: http://git.kernel.org/linus/3383977

Oooh, right. I for some reason thought block layer was already doing that. I'll update it. Thanks for pointing it out.

Quoted 3 lines

Doesn't seem like we need to do the same for REQ_FLUSH (at least not for rq-based DM's benefit) because dm.c:setup_clone already special cases flush requests and sets REQ_FLUSH in cmd_flags.

Hmmm... but in general, I think it would be better to make __blk_rq_prep_clone() to copy all command related bits. If some command bits shouldn't be copied, the caller should take care of them.

Thanks.

Signature · 1 line
tejun
#29 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

In reply to Tejun Heo

Hi Tejun, Mike,

Quoted 8 lines · Tejun Heo
Quoted 4 lines · Mike Snitzer

NOTE: NEC has already given some preliminary feedback to hch in the "[PATCH, RFC 2/2] dm: support REQ_FLUSH directly" thread:
https://www.redhat.com/archives/dm-devel/2010-August/msg00026.html
https://www.redhat.com/archives/dm-devel/2010-August/msg00033.html

Hmmm... I think both issues don't exist in this incarnation of conversion although I'm fairly sure there will be other issues. :-)

The same issue is still there for request-based dm. See below.

Quoted 9 lines
Quoted 8 lines
Quoted 3 lines

A related question: Is dm_wait_for_completion() used in process_flush() safe against starvation under continuous influx of other commands?

As for your specific dm_wait_for_completion() concern -- I'll defer to Mikulas. But I'll add: we haven't had any reported starvation issues with DM's existing barrier support. DM uses a mempool for its clones, so it should naturally throttle (without starvation) when memory gets low.

Quoted 3 lines

I see but single pending flush and steady write streams w/o saturating the mempool would be able to stall dm_wait_for_completeion(), no? Eh well, it's a separate issue, I guess.

Your understanding is correct, dm_wait_for_completion() for flush will stall in such cases for request-based dm.
That's why I mentioned below in
https://www.redhat.com/archives/dm-devel/2010-August/msg00026.html.

In other words, current request-based device-mapper can't handle other requests while a flush request is in progress.

In flush request handling, request-based dm uses dm_wait_for_completion() to wait for the completion of cloned flush requests, depending on the fact that there should be only flush requests in flight owning to the block layer sequencing.

It's not a separate issue and we need to resolve it at least. I'm still considering how I can fix the request-based dm.

Thanks,
Kiyoshi Ueda

author
#30 lore raw

Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

Hello,

Quoted 9 lines · Kiyoshi Ueda
Quoted 3 lines

I see but single pending flush and steady write streams w/o saturating the mempool would be able to stall dm_wait_for_completeion(), no? Eh well, it's a separate issue, I guess.

Your understanding is correct, dm_wait_for_completion() for flush will stall in such cases for request-based dm.
That's why I mentioned below in
https://www.redhat.com/archives/dm-devel/2010-August/msg00026.html.

Quoted 7 lines

In other words, current request-based device-mapper can't handle other requests while a flush request is in progress.

In flush request handling, request-based dm uses dm_wait_for_completion() to wait for the completion of cloned flush requests, depending on the fact that there should be only flush requests in flight owning to the block layer sequencing.

I see. bio based implementation also uses dm_wait_for_completion() but it also has DMF_QUEUE_IO_TO_THREAD to plug all the follow up bio's while flush is in progress, which sucks for throughput but successfully avoids starvation.

Quoted 2 lines

It's not a separate issue and we need to resolve it at least. I'm still considering how I can fix the request-based dm.

Right, I thought you were talking about REQ_FLUSHes not sycnhronized against barrier write. Anyways, yeah, it's a problem. I don't think not being able to handle multiple flushes concurrently would be a major issue. The problem is not being able to process other bios/requests while a flush is in progress. All that's necessary is making the completion detection a bit more fine grained so that it counts the number of in flight flush bios/requests and completes when it reaches zero instead of waiting for all outstanding commands. Shouldn't be too hard.

Thanks.

Signature · 1 line
tejun
author
#31 lore raw

PATCH 5/5 dm: implement REQ_FLUSH/FUA support

In reply to Tejun Heo

From: Tejun Heo <[email protected]>

This patch converts dm to support REQ_FLUSH/FUA instead of now deprecated REQ_HARDBARRIER.

For common parts,

* Barrier retry logic dropped from dm-io.

* Empty WRITE_BARRIERs are replaced with WRITE_FLUSHes.

* It's now guaranteed that all FLUSH bio's which are passed onto dm targets are zero length. bio_empty_barrier() tests are replaced with REQ_FLUSH tests.

* Dropped unlikely() around REQ_FLUSH tests. Flushes are not unlikely enough to be marked with unlikely().

For bio-based dm,

* Preflush is handled as before but postflush is dropped and replaced with passing down REQ_FUA to member request_queues. This replaces one array wide cache flush w/ member specific FUA writes.

* __split_and_process_bio() now calls __clone_and_map_flush() directly for flushes and guarantees all FLUSH bio's going to targets are zero length.

* -EOPNOTSUPP retry logic dropped.

For request-based dm,

* Nothing much changes. It just needs to handle FLUSH requests as before. It would be beneficial to advertise FUA capability so that it can propagate FUA flags down to member request_queues instead of sequencing it as WRITE + FLUSH at the top queue.

Lightly tested linear, stripe, raid1, snap and crypt targets. Please proceed with caution as I'm not familiar with the code base.

---
 drivers/md/dm-crypt.c           |    2 +-
 drivers/md/dm-io.c              |   20 +----
 drivers/md/dm-log.c             |    2 +-
 drivers/md/dm-raid1.c           |    8 +-
 drivers/md/dm-region-hash.c     |   16 ++--
 drivers/md/dm-snap-persistent.c |    2 +-
 drivers/md/dm-snap.c            |    6 +-
 drivers/md/dm-stripe.c          |    2 +-
 drivers/md/dm.c                 |  180 +++++++++++++++++++-------------------
 9 files changed, 113 insertions(+), 125 deletions(-)
drivers/md/dm-crypt.c +1 -1
index 3bdbb61..da11652 100644
@@ -1249,7 +1249,7 @@ static int crypt_map(struct dm_target *ti, struct bio *bio,
 	struct dm_crypt_io *io;
 	struct crypt_config *cc;
 
-	if (unlikely(bio_empty_barrier(bio))) {
+	if (bio->bi_rw & REQ_FLUSH) {
 		cc = ti->private;
 		bio->bi_bdev = cc->dev->bdev;
 		return DM_MAPIO_REMAPPED;
drivers/md/dm-io.c +4 -16
index 0590c75..136d4f7 100644
@@ -31,7 +31,6 @@ struct dm_io_client {
  */
 struct io {
 	unsigned long error_bits;
-	unsigned long eopnotsupp_bits;
 	atomic_t count;
 	struct task_struct *sleeper;
 	struct dm_io_client *client;
@@ -130,11 +129,8 @@ static void retrieve_io_and_region_from_bio(struct bio *bio, struct io **io,
  *---------------------------------------------------------------*/
 static void dec_count(struct io *io, unsigned int region, int error)
 {
-	if (error) {
+	if (error)
 		set_bit(region, &io->error_bits);
-		if (error == -EOPNOTSUPP)
-			set_bit(region, &io->eopnotsupp_bits);
-	}
 
 	if (atomic_dec_and_test(&io->count)) {
 		if (io->sleeper)
@@ -310,8 +306,8 @@ static void do_region(int rw, unsigned region, struct dm_io_region *where,
 	sector_t remaining = where->count;
 
 	/*
-	 * where->count may be zero if rw holds a write barrier and we
-	 * need to send a zero-sized barrier.
+	 * where->count may be zero if rw holds a flush and we need to
+	 * send a zero-sized flush.
 	 */
 	do {
 		/*
@@ -364,7 +360,7 @@ static void dispatch_io(int rw, unsigned int num_regions,
 	 */
 	for (i = 0; i < num_regions; i++) {
 		*dp = old_pages;
-		if (where[i].count || (rw & REQ_HARDBARRIER))
+		if (where[i].count || (rw & REQ_FLUSH))
 			do_region(rw, i, where + i, dp, io);
 	}
 
@@ -393,9 +389,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
 		return -EIO;
 	}
 
-retry:
 	io->error_bits = 0;
-	io->eopnotsupp_bits = 0;
 	atomic_set(&io->count, 1); /* see dispatch_io() */
 	io->sleeper = current;
 	io->client = client;
@@ -412,11 +406,6 @@ retry:
 	}
 	set_current_state(TASK_RUNNING);
 
-	if (io->eopnotsupp_bits && (rw & REQ_HARDBARRIER)) {
-		rw &= ~REQ_HARDBARRIER;
-		goto retry;
-	}
-
 	if (error_bits)
 		*error_bits = io->error_bits;
 
@@ -437,7 +426,6 @@ static int async_io(struct dm_io_client *client, unsigned int num_regions,
 
 	io = mempool_alloc(client->pool, GFP_NOIO);
 	io->error_bits = 0;
-	io->eopnotsupp_bits = 0;
 	atomic_set(&io->count, 1); /* see dispatch_io() */
 	io->sleeper = NULL;
 	io->client = client;
drivers/md/dm-log.c +1 -1
index 5a08be0..33420e6 100644
@@ -300,7 +300,7 @@ static int flush_header(struct log_c *lc)
 		.count = 0,
 	};
 
-	lc->io_req.bi_rw = WRITE_BARRIER;
+	lc->io_req.bi_rw = WRITE_FLUSH;
 
 	return dm_io(&lc->io_req, 1, &null_location, NULL);
 }
drivers/md/dm-raid1.c +4 -4
index 7413626..af33245 100644
@@ -259,7 +259,7 @@ static int mirror_flush(struct dm_target *ti)
 	struct dm_io_region io[ms->nr_mirrors];
 	struct mirror *m;
 	struct dm_io_request io_req = {
-		.bi_rw = WRITE_BARRIER,
+		.bi_rw = WRITE_FLUSH,
 		.mem.type = DM_IO_KMEM,
 		.mem.ptr.bvec = NULL,
 		.client = ms->io_client,
@@ -629,7 +629,7 @@ static void do_write(struct mirror_set *ms, struct bio *bio)
 	struct dm_io_region io[ms->nr_mirrors], *dest = io;
 	struct mirror *m;
 	struct dm_io_request io_req = {
-		.bi_rw = WRITE | (bio->bi_rw & WRITE_BARRIER),
+		.bi_rw = WRITE | (bio->bi_rw & WRITE_FLUSH_FUA),
 		.mem.type = DM_IO_BVEC,
 		.mem.ptr.bvec = bio->bi_io_vec + bio->bi_idx,
 		.notify.fn = write_callback,
@@ -670,7 +670,7 @@ static void do_writes(struct mirror_set *ms, struct bio_list *writes)
 	bio_list_init(&requeue);
 
 	while ((bio = bio_list_pop(writes))) {
-		if (unlikely(bio_empty_barrier(bio))) {
+		if (bio->bi_rw & REQ_FLUSH) {
 			bio_list_add(&sync, bio);
 			continue;
 		}
@@ -1203,7 +1203,7 @@ static int mirror_end_io(struct dm_target *ti, struct bio *bio,
 	 * We need to dec pending if this was a write.
 	 */
 	if (rw == WRITE) {
-		if (likely(!bio_empty_barrier(bio)))
+		if (!(bio->bi_rw & REQ_FLUSH))
 			dm_rh_dec(ms->rh, map_context->ll);
 		return error;
 	}
drivers/md/dm-region-hash.c +8 -8
index bd5c58b..dad011a 100644
@@ -81,9 +81,9 @@ struct dm_region_hash {
 	struct list_head failed_recovered_regions;
 
 	/*
-	 * If there was a barrier failure no regions can be marked clean.
+	 * If there was a flush failure no regions can be marked clean.
 	 */
-	int barrier_failure;
+	int flush_failure;
 
 	void *context;
 	sector_t target_begin;
@@ -217,7 +217,7 @@ struct dm_region_hash *dm_region_hash_create(
 	INIT_LIST_HEAD(&rh->quiesced_regions);
 	INIT_LIST_HEAD(&rh->recovered_regions);
 	INIT_LIST_HEAD(&rh->failed_recovered_regions);
-	rh->barrier_failure = 0;
+	rh->flush_failure = 0;
 
 	rh->region_pool = mempool_create_kmalloc_pool(MIN_REGIONS,
 						      sizeof(struct dm_region));
@@ -399,8 +399,8 @@ void dm_rh_mark_nosync(struct dm_region_hash *rh, struct bio *bio)
 	region_t region = dm_rh_bio_to_region(rh, bio);
 	int recovering = 0;
 
-	if (bio_empty_barrier(bio)) {
-		rh->barrier_failure = 1;
+	if (bio->bi_rw & REQ_FLUSH) {
+		rh->flush_failure = 1;
 		return;
 	}
 
@@ -524,7 +524,7 @@ void dm_rh_inc_pending(struct dm_region_hash *rh, struct bio_list *bios)
 	struct bio *bio;
 
 	for (bio = bios->head; bio; bio = bio->bi_next) {
-		if (bio_empty_barrier(bio))
+		if (bio->bi_rw & REQ_FLUSH)
 			continue;
 		rh_inc(rh, dm_rh_bio_to_region(rh, bio));
 	}
@@ -555,9 +555,9 @@ void dm_rh_dec(struct dm_region_hash *rh, region_t region)
 		 */
 
 		/* do nothing for DM_RH_NOSYNC */
-		if (unlikely(rh->barrier_failure)) {
+		if (unlikely(rh->flush_failure)) {
 			/*
-			 * If a write barrier failed some time ago, we
+			 * If a write flush failed some time ago, we
 			 * don't know whether or not this write made it
 			 * to the disk, so we must resync the device.
 			 */
drivers/md/dm-snap-persistent.c +1 -1
index c097d8a..b9048f0 100644
@@ -687,7 +687,7 @@ static void persistent_commit_exception(struct dm_exception_store *store,
 	/*
 	 * Commit exceptions to disk.
 	 */
-	if (ps->valid && area_io(ps, WRITE_BARRIER))
+	if (ps->valid && area_io(ps, WRITE_FLUSH_FUA))
 		ps->valid = 0;
 
 	/*
drivers/md/dm-snap.c +3 -3
index 5485377..62a6586 100644
@@ -1581,7 +1581,7 @@ static int snapshot_map(struct dm_target *ti, struct bio *bio,
 	chunk_t chunk;
 	struct dm_snap_pending_exception *pe = NULL;
 
-	if (unlikely(bio_empty_barrier(bio))) {
+	if (bio->bi_rw & REQ_FLUSH) {
 		bio->bi_bdev = s->cow->bdev;
 		return DM_MAPIO_REMAPPED;
 	}
@@ -1685,7 +1685,7 @@ static int snapshot_merge_map(struct dm_target *ti, struct bio *bio,
 	int r = DM_MAPIO_REMAPPED;
 	chunk_t chunk;
 
-	if (unlikely(bio_empty_barrier(bio))) {
+	if (bio->bi_rw & REQ_FLUSH) {
 		if (!map_context->flush_request)
 			bio->bi_bdev = s->origin->bdev;
 		else
@@ -2123,7 +2123,7 @@ static int origin_map(struct dm_target *ti, struct bio *bio,
 	struct dm_dev *dev = ti->private;
 	bio->bi_bdev = dev->bdev;
 
-	if (unlikely(bio_empty_barrier(bio)))
+	if (bio->bi_rw & REQ_FLUSH)
 		return DM_MAPIO_REMAPPED;
 
 	/* Only tell snapshots if this is a write */
drivers/md/dm-stripe.c +1 -1
index d6e28d7..0f534ca 100644
@@ -214,7 +214,7 @@ static int stripe_map(struct dm_target *ti, struct bio *bio,
 	sector_t offset, chunk;
 	uint32_t stripe;
 
-	if (unlikely(bio_empty_barrier(bio))) {
+	if (bio->bi_rw & REQ_FLUSH) {
 		BUG_ON(map_context->flush_request >= sc->stripes);
 		bio->bi_bdev = sc->stripe[map_context->flush_request].dev->bdev;
 		return DM_MAPIO_REMAPPED;
drivers/md/dm.c +90 -90

425 lines not on this page · raw message

Signature · 1 line
1.7.1
#32 lore raw
In reply to Tejun Heo
Quoted 2 lines · Tejun Heo

* scsi_error.c for some reason tests REQ_HARDBARRIER. I think this part can be dropped altogether but am not sure.

That was introduced by commit 77a4229719e511a0d38d9c355317ae1469adeb54

[SCSI] Retry commands with UNIT_ATTENTION sense codes to fix ext3/ext4 I/O error

which just went in this May. Given that we now send barriers and discards as FS requests I think it should be reverted, preferably already during this cycle, in which the patches to send these requests as FS type were added.

#33 lore raw
Quoted 4 lines · Christoph Hellwig
Quoted 2 lines · Tejun Heo

* scsi_error.c for some reason tests REQ_HARDBARRIER. I think this part can be dropped altogether but am not sure.

Quoted 8 lines

That was introduced by commit 77a4229719e511a0d38d9c355317ae1469adeb54

[SCSI] Retry commands with UNIT_ATTENTION sense codes to fix ext3/ext4 I/O error

which just went in this May. Given that we now send barriers and discards as FS requests I think it should be reverted, preferably already during this cycle, in which the patches to send these requests as FS type were added.

Right ... it was the quickest hack to get around the fact that we expect the caller to do error handling for BLOCK_PC requests and none of the block discard and barrier code expects to do this.

Once they all go as REQ_TYPE_FS, this code can be removed.

James

#34 lore raw
Quoted 5 lines · James Bottomley

Right ... it was the quickest hack to get around the fact that we expect the caller to do error handling for BLOCK_PC requests and none of the block discard and barrier code expects to do this.

Once they all go as REQ_TYPE_FS, this code can be removed.

In current mainline all discard and barrier requests are REQ_TYPE_FS.

#35 lore raw
In reply to James Bottomley
Quoted 14 lines · On Wed, 18 Aug 2010 09:26:27 -0500 James Bottomley <[email protected]>…
Quoted 4 lines · Christoph Hellwig
Quoted 2 lines · Tejun Heo

* scsi_error.c for some reason tests REQ_HARDBARRIER. I think this part can be dropped altogether but am not sure.

Quoted 8 lines

That was introduced by commit 77a4229719e511a0d38d9c355317ae1469adeb54

[SCSI] Retry commands with UNIT_ATTENTION sense codes to fix ext3/ext4 I/O error

which just went in this May. Given that we now send barriers and discards as FS requests I think it should be reverted, preferably already during this cycle, in which the patches to send these requests as FS type were added.

Quoted 5 lines

Right ... it was the quickest hack to get around the fact that we expect the caller to do error handling for BLOCK_PC requests and none of the block discard and barrier code expects to do this.

Once they all go as REQ_TYPE_FS, this code can be removed.

I think that I reverted it with the discard and flush conversion. No idea why the code exists in the current git.

commit e96f6abe02fc3320d669985443e8c68ff8e83294

Author: FUJITA Tomonori <[email protected]>
Date:   Fri Jul 9 09:38:26 2010 +0900

scsi-ml uses REQ_TYPE_BLOCK_PC for flush requests from file systems. The definition of REQ_TYPE_BLOCK_PC is that we don't retry requests even when we can (e.g. UNIT ATTENTION) and we send the response to the callers (then the callers can decide what they want). We need a workaround such as the commit 77a4229719e511a0d38d9c355317ae1469adeb54 to retry BLOCK_PC flush requests. We will need the similar workaround for discard requests too since SCSI-ml handle them as BLOCK_PC internally.

This uses REQ_TYPE_FS for flush requests from file systems instead of REQ_TYPE_BLOCK_PC.

scsi-ml retries only REQ_TYPE_FS requests that have data to transfer when we can retry them (e.g. UNIT_ATTENTION). However, we also need to retry REQ_TYPE_FS requests without data because the callers don't.

This also changes scsi_check_sense() to retry all the REQ_TYPE_FS requests when appropriate. Thanks to scsi_noretry_cmd(), REQ_TYPE_BLOCK_PC requests don't be retried as before.

Note that basically, this reverts the commit 77a4229719e511a0d38d9c355317ae1469adeb54 since now we use REQ_TYPE_FS for flush requests.

---
 drivers/scsi/scsi_error.c |   19 ++++---------------
 drivers/scsi/sd.c         |    2 --
 2 files changed, 4 insertions(+), 17 deletions(-)
drivers/scsi/scsi_error.c +4 -15
index 1b88af8..2768bf6 100644
@@ -307,20 +307,7 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 		    (sshdr.asc == 0x04) && (sshdr.ascq == 0x02))
 			return FAILED;
 
-		if (scmd->request->cmd_flags & REQ_HARDBARRIER)
-			/*
-			 * barrier requests should always retry on UA
-			 * otherwise block will get a spurious error
-			 */
-			return NEEDS_RETRY;
-		else
-			/*
-			 * for normal (non barrier) commands, pass the
-			 * UA upwards for a determination in the
-			 * completion functions
-			 */
-			return SUCCESS;
-
+		return NEEDS_RETRY;
 		/* these three are not supported */
 	case COPY_ABORTED:
 	case VOLUME_OVERFLOW:
@@ -1336,7 +1323,9 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd)
 		 * assume caller has checked sense and determinted
 		 * the check condition was retryable.
 		 */
-		return (scmd->request->cmd_flags & REQ_FAILFAST_DEV);
+		if (scmd->request->cmd_flags & REQ_FAILFAST_DEV ||
+		    scmd->request->cmd_type == REQ_TYPE_BLOCK_PC)
+			return 1;
 	}
 
 	return 0;
drivers/scsi/sd.c +0 -2
index e63b85a..108daea 100644
@@ -477,8 +477,6 @@ static int scsi_setup_discard_cmnd(struct scsi_device *sdp, struct request *rq)
 
 static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
 {
-	/* for now, we use REQ_TYPE_BLOCK_PC. */
-	rq->cmd_type = REQ_TYPE_BLOCK_PC;
 	rq->timeout = SD_TIMEOUT;
 	rq->retries = SD_MAX_RETRIES;
 	rq->cmd[0] = SYNCHRONIZE_CACHE;
Signature · 1 line
1.6.5
#36 lore raw
Quoted 2 lines · FUJITA Tomonori

I think that I reverted it with the discard and flush conversion. No idea why the code exists in the current git.

Given that this:

drivers/scsi/scsi_error.c static int scsi_check_sense(struct scsi_cmnd *scmd) · line 310

Quoted 1 line
drivers/scsi/scsi_error.c +0 -1
-		if (scmd->request->cmd_flags & REQ_HARDBARRIER)

line is attributed to a merge commit by Jens in Linus' current git tree it might have been a merge error of some sorts.

#37 lore raw
Quoted 10 lines · On Thu, 19 Aug 2010 17:41:33 +0200 Christoph Hellwig <[email protected]> wrote:
Quoted 2 lines · FUJITA Tomonori

I think that I reverted it with the discard and flush conversion. No idea why the code exists in the current git.

Given that this:

Review

Quoted 1 line
(quoted diff) +0 -1
-		if (scmd->request->cmd_flags & REQ_HARDBARRIER)

line is attributed to a merge commit by Jens in Linus' current git tree it might have been a merge error of some sorts.

Yeah, looks like so.

Anyway, we can remove the workaround safely now.

=
From: FUJITA Tomonori <[email protected]>
Date: Fri, 20 Aug 2010 00:47:45 +0900
Subject: [PATCH] fix the merge error of UNIT_ATTENTION workaround removal

The commit e96f6abe02fc3320d669985443e8c68ff8e83294 reverted the commit 77a4229719e511a0d38d9c355317ae1469adeb54, UNIT_ATTENTION workaround for barriers with the barrier and discard FS request conversion. But due to a merge error, the workaround still exists.

---
 drivers/scsi/scsi_error.c |   15 +--------------
 1 files changed, 1 insertions(+), 14 deletions(-)
drivers/scsi/scsi_error.c +1 -14
index 1de30eb..5743196 100644
@@ -320,20 +320,7 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 				    "changed. The Linux SCSI layer does not "
 				    "automatically adjust these parameters.\n");
 
-		if (scmd->request->cmd_flags & REQ_HARDBARRIER)
-			/*
-			 * barrier requests should always retry on UA
-			 * otherwise block will get a spurious error
-			 */
-			return NEEDS_RETRY;
-		else
-			/*
-			 * for normal (non barrier) commands, pass the
-			 * UA upwards for a determination in the
-			 * completion functions
-			 */
-			return SUCCESS;
-
+		return NEEDS_RETRY;
 		/* these three are not supported */
 	case COPY_ABORTED:
 	case VOLUME_OVERFLOW:
Signature · 1 line
1.6.5
#39 lore raw
Quoted 11 lines · Christoph Hellwig

Phillipp and Lars,

can you look at the changes in

	http://www.spinics.net/lists/linux-fsdevel/msg35884.html

and

	http://www.spinics.net/lists/linux-fsdevel/msg36001.html

and see how drbd can be adapted to it?

Yes of course.
I have been closely following the discussion related to this patchset.

Quoted 3 lines

It's one of two drivers still
missing, and because it's not quite intuitive what it's doing it's very hard for Tejun and me to help.

I'm currently on vacation, actually, and Phil is likely very busy, but we should have a patche ready within the next few days anyways.

Signature · 5 lines
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com

DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
#40 lore raw
In reply to Christoph Hellwig

Am Montag, 23. August 2010, um 18:47:09 schrieb Christoph Hellwig:

Quoted 6 lines

Phillipp and Lars,

can you look at the changes in

	http://www.spinics.net/lists/linux-fsdevel/msg35884.html
Quoted 7 lines

and

	http://www.spinics.net/lists/linux-fsdevel/msg36001.html

and see how drbd can be adapted to it? It's one of two drivers still missing, and because it's not quite intuitive what it's doing it's very hard for Tejun and me to help.

Hi Christoph,

I was able to finish the greater part of the necessary work today. You can find it there:

git://git.drbd.org/linux-2.6-drbd.git flush-fua

Of course that is based on

git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git flush-fua

REQ_HARDBARRIER is not yet completely eliminated from drbd, but I am very confident that I will be able to finish that by tomorrow.

Best,
Phil

Signature · 6 lines
: Dipl-Ing Philipp Reisner
: LINBIT | Your Way to High Availability
: Tel: +43-1-8178292-50, Fax: +43-1-8178292-82
: http://www.linbit.com

DRBD(R) and LINBIT(R) are registered trademarks of LINBIT, Austria.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#42 lore raw

[GIT PULL] convert DRBD to REQ_FLUSH/FUA

Hi Jens,

Christoph Hellwig pointed out that drbd is the only driver that still uses/supports barriers instead of FUE in linux-next.

I revitalized three patches that remove the barrier bits from DRBD...

The following changes since commit 8825f7c3e5c7b251b49fc594658a96f59417ee16:

drbd: Silenced an assert (2010-10-22 15:55:22 +0200)

are available in the git repository at:
git://git.drbd.org/linux-2.6-drbd.git for-jens

Philipp Reisner (3):
drbd: Removed the BIO_RW_BARRIER support form the receiver/epoch code
drbd: REQ_HARDBARRIER -> REQ_FUA transition for meta data accesses
drbd: Removed checks for REQ_HARDBARRIER on incomming BIOs

 drivers/block/drbd/drbd_actlog.c   |   16 +---
 drivers/block/drbd/drbd_int.h      |   20 +---
 drivers/block/drbd/drbd_main.c     |    7 +-
 drivers/block/drbd/drbd_nl.c       |    8 +-
 drivers/block/drbd/drbd_proc.c     |    1 -
 drivers/block/drbd/drbd_receiver.c |  212 +++++-------------------------------
 drivers/block/drbd/drbd_req.c      |   14 ---
 drivers/block/drbd/drbd_worker.c   |   21 ----
 8 files changed, 40 insertions(+), 259 deletions(-)
#43 lore raw

Re: [GIT PULL] convert DRBD to REQ_FLUSH/FUA

Quoted 6 lines · Philipp Reisner

Hi Jens,

Christoph Hellwig pointed out that drbd is the only driver that still uses/supports barriers instead of FUE in linux-next.

I revitalized three patches that remove the barrier bits from DRBD...

This looks good. You probably don't actually need the MD_NO_FUA flag - REQ_FUA writes will always work - they just ignored by drivers not implementing cache control.

#44 lore raw

Re: [GIT PULL] convert DRBD to REQ_FLUSH/FUA

In reply to Philipp Reisner
Quoted 19 lines · Philipp Reisner

Hi Jens,

Christoph Hellwig pointed out that drbd is the only driver that still uses/supports barriers instead of FUE in linux-next.

I revitalized three patches that remove the barrier bits from DRBD...

The following changes since commit 8825f7c3e5c7b251b49fc594658a96f59417ee16:

drbd: Silenced an assert (2010-10-22 15:55:22 +0200)

are available in the git repository at:
git://git.drbd.org/linux-2.6-drbd.git for-jens

Philipp Reisner (3):
drbd: Removed the BIO_RW_BARRIER support form the receiver/epoch code
drbd: REQ_HARDBARRIER -> REQ_FUA transition for meta data accesses
drbd: Removed checks for REQ_HARDBARRIER on incomming BIOs

Quoted 9 lines
 drivers/block/drbd/drbd_actlog.c   |   16 +---
 drivers/block/drbd/drbd_int.h      |   20 +---
 drivers/block/drbd/drbd_main.c     |    7 +-
 drivers/block/drbd/drbd_nl.c       |    8 +-
 drivers/block/drbd/drbd_proc.c     |    1 -
 drivers/block/drbd/drbd_receiver.c |  212 +++++-------------------------------
 drivers/block/drbd/drbd_req.c      |   14 ---
 drivers/block/drbd/drbd_worker.c   |   21 ----
 8 files changed, 40 insertions(+), 259 deletions(-)

Thanks, pulled!

Signature · 1 line
Jens Axboe
#45 lore raw

PATCH 1/1 block: remove REQ_HARDBARRIER

REQ_HARDBARRIER is dead now, so remove the leftovers. What's left at this point is:

- various checks inside the block layer.
- sanity checks in bio based drivers.
- now unused bio_empty_barrier helper.
- Xen blockfront use of BLKIF_OP_WRITE_BARRIER - it's dead for a while, but Xen really needs to sort out it's barrier situaton.
- setting of ordered tags in uas - dead code copied from old scsi drivers.
- scsi different retry for barriers - it's dead and should have been removed when flushes were converted to FS requests.
- blktrace handling of barriers - removed. Someone who knows blktrace better should add support for REQ_FLUSH and REQ_FUA, though.

block/blk-core.c +0 -7
Index: linux-2.6/block/blk-core.c
===================================================================
@@ -1204,13 +1204,6 @@ static int __make_request(struct request
 	int where = ELEVATOR_INSERT_SORT;
 	int rw_flags;
 
-	/* REQ_HARDBARRIER is no more */
-	if (WARN_ONCE(bio->bi_rw & REQ_HARDBARRIER,
-		"block: HARDBARRIER is deprecated, use FLUSH/FUA instead\n")) {
-		bio_endio(bio, -EOPNOTSUPP);
-		return 0;
-	}
-
 	/*
 	 * low level driver can indicate that it wants pages above a
 	 * certain limit bounced to low memory (ie for highmem, or even
block/elevator.c +2 -2
Index: linux-2.6/block/elevator.c
===================================================================
@@ -429,7 +429,7 @@ void elv_dispatch_sort(struct request_qu
 	q->nr_sorted--;
 
 	boundary = q->end_sector;
-	stop_flags = REQ_SOFTBARRIER | REQ_HARDBARRIER | REQ_STARTED;
+	stop_flags = REQ_SOFTBARRIER | REQ_STARTED;
 	list_for_each_prev(entry, &q->queue_head) {
 		struct request *pos = list_entry_rq(entry);
 
@@ -691,7 +691,7 @@ void elv_insert(struct request_queue *q,
 void __elv_add_request(struct request_queue *q, struct request *rq, int where,
 		       int plug)
 {
-	if (rq->cmd_flags & (REQ_SOFTBARRIER | REQ_HARDBARRIER)) {
+	if (rq->cmd_flags & REQ_SOFTBARRIER) {
 		/* barriers are scheduling boundary, update end_sector */
 		if (rq->cmd_type == REQ_TYPE_FS ||
 		    (rq->cmd_flags & REQ_DISCARD)) {
drivers/block/aoe/aoeblk.c +0 -3
Index: linux-2.6/drivers/block/aoe/aoeblk.c
===================================================================
@@ -178,9 +178,6 @@ aoeblk_make_request(struct request_queue
 		BUG();
 		bio_endio(bio, -ENXIO);
 		return 0;
-	} else if (bio->bi_rw & REQ_HARDBARRIER) {
-		bio_endio(bio, -EOPNOTSUPP);
-		return 0;
 	} else if (bio->bi_io_vec == NULL) {
 		printk(KERN_ERR "aoe: bi_io_vec is NULL\n");
 		BUG();
drivers/block/loop.c +0 -6
Index: linux-2.6/drivers/block/loop.c
===================================================================
@@ -481,12 +481,6 @@ static int do_bio_filebacked(struct loop
 	if (bio_rw(bio) == WRITE) {
 		struct file *file = lo->lo_backing_file;
 
-		/* REQ_HARDBARRIER is deprecated */
-		if (bio->bi_rw & REQ_HARDBARRIER) {
-			ret = -EOPNOTSUPP;
-			goto out;
-		}
-
 		if (bio->bi_rw & REQ_FLUSH) {
 			ret = vfs_fsync(file, 0);
 			if (unlikely(ret && ret != -EINVAL)) {
drivers/block/xen-blkfront.c +0 -2
Index: linux-2.6/drivers/block/xen-blkfront.c
===================================================================
@@ -289,8 +289,6 @@ static int blkif_queue_request(struct re
 
 	ring_req->operation = rq_data_dir(req) ?
 		BLKIF_OP_WRITE : BLKIF_OP_READ;
-	if (req->cmd_flags & REQ_HARDBARRIER)
-		ring_req->operation = BLKIF_OP_WRITE_BARRIER;
 
 	ring_req->nr_segments = blk_rq_map_sg(req->q, req, info->sg);
 	BUG_ON(ring_req->nr_segments > BLKIF_MAX_SEGMENTS_PER_REQUEST);
drivers/scsi/scsi_error.c +5 -13
Index: linux-2.6/drivers/scsi/scsi_error.c
===================================================================
@@ -320,19 +320,11 @@ static int scsi_check_sense(struct scsi_
 				    "changed. The Linux SCSI layer does not "
 				    "automatically adjust these parameters.\n");
 
-		if (scmd->request->cmd_flags & REQ_HARDBARRIER)
-			/*
-			 * barrier requests should always retry on UA
-			 * otherwise block will get a spurious error
-			 */
-			return NEEDS_RETRY;
-		else
-			/*
-			 * for normal (non barrier) commands, pass the
-			 * UA upwards for a determination in the
-			 * completion functions
-			 */
-			return SUCCESS;
+		/*
+		 * Pass the UA upwards for a determination in the completion
+		 * functions.
+		 */
+		return SUCCESS;
 
 		/* these three are not supported */
 	case COPY_ABORTED:
drivers/usb/storage/uas.c +1 -4
Index: linux-2.6/drivers/usb/storage/uas.c
===================================================================
@@ -331,10 +331,7 @@ static struct urb *uas_alloc_cmd_urb(str
 
 	iu->iu_id = IU_ID_COMMAND;
 	iu->tag = cpu_to_be16(stream_id);
-	if (sdev->ordered_tags && (cmnd->request->cmd_flags & REQ_HARDBARRIER))
-		iu->prio_attr = UAS_ORDERED_TAG;
-	else
-		iu->prio_attr = UAS_SIMPLE_TAG;
+	iu->prio_attr = UAS_SIMPLE_TAG;
 	iu->len = len;
 	int_to_scsilun(sdev->lun, &iu->lun);
 	memcpy(iu->cdb, cmnd->cmnd, cmnd->cmd_len);
include/linux/bio.h +0 -4
Index: linux-2.6/include/linux/bio.h
===================================================================
@@ -66,10 +66,6 @@
 #define bio_offset(bio)		bio_iovec((bio))->bv_offset
 #define bio_segments(bio)	((bio)->bi_vcnt - (bio)->bi_idx)
 #define bio_sectors(bio)	((bio)->bi_size >> 9)
-#define bio_empty_barrier(bio) \
-	((bio->bi_rw & REQ_HARDBARRIER) && \
-	 !bio_has_data(bio) && \
-	 !(bio->bi_rw & REQ_DISCARD))
 
 static inline unsigned int bio_cur_bytes(struct bio *bio)
 {
include/linux/blk_types.h +2 -4
Index: linux-2.6/include/linux/blk_types.h
===================================================================
@@ -122,7 +122,6 @@ enum rq_flag_bits {
 	__REQ_FAILFAST_TRANSPORT, /* no driver retries of transport errors */
 	__REQ_FAILFAST_DRIVER,	/* no driver retries of driver errors */
 
-	__REQ_HARDBARRIER,	/* may not be passed by drive either */
 	__REQ_SYNC,		/* request is sync (sync write or read) */
 	__REQ_META,		/* metadata io request */
 	__REQ_DISCARD,		/* request to discard sectors */
@@ -159,7 +158,6 @@ enum rq_flag_bits {
 #define REQ_FAILFAST_DEV	(1 << __REQ_FAILFAST_DEV)
 #define REQ_FAILFAST_TRANSPORT	(1 << __REQ_FAILFAST_TRANSPORT)
 #define REQ_FAILFAST_DRIVER	(1 << __REQ_FAILFAST_DRIVER)
-#define REQ_HARDBARRIER		(1 << __REQ_HARDBARRIER)
 #define REQ_SYNC		(1 << __REQ_SYNC)
 #define REQ_META		(1 << __REQ_META)
 #define REQ_DISCARD		(1 << __REQ_DISCARD)
@@ -168,8 +166,8 @@ enum rq_flag_bits {
 #define REQ_FAILFAST_MASK \
 	(REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER)
 #define REQ_COMMON_MASK \
-	(REQ_WRITE | REQ_FAILFAST_MASK | REQ_HARDBARRIER | REQ_SYNC | \
-	 REQ_META | REQ_DISCARD | REQ_NOIDLE | REQ_FLUSH | REQ_FUA)
+	(REQ_WRITE | REQ_FAILFAST_MASK | REQ_SYNC | REQ_META | REQ_DISCARD | \
+	 REQ_NOIDLE | REQ_FLUSH | REQ_FUA)
 #define REQ_CLONE_MASK		REQ_COMMON_MASK
 
 #define REQ_UNPLUG		(1 << __REQ_UNPLUG)
include/linux/blkdev.h +1 -2
Index: linux-2.6/include/linux/blkdev.h
===================================================================
@@ -553,8 +553,7 @@ static inline void blk_clear_queue_full(
  * it already be started by driver.
  */
 #define RQ_NOMERGE_FLAGS	\
-	(REQ_NOMERGE | REQ_STARTED | REQ_HARDBARRIER | REQ_SOFTBARRIER | \
-	 REQ_FLUSH | REQ_FUA)
+	(REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA)
 #define rq_mergeable(rq)	\
 	(!((rq)->cmd_flags & RQ_NOMERGE_FLAGS) && \
 	 (((rq)->cmd_flags & REQ_DISCARD) || \
kernel/trace/blktrace.c +0 -4
Index: linux-2.6/kernel/trace/blktrace.c
===================================================================
@@ -168,7 +168,6 @@ static int act_log_check(struct blk_trac
 static const u32 ddir_act[2] = { BLK_TC_ACT(BLK_TC_READ),
 				 BLK_TC_ACT(BLK_TC_WRITE) };
 
-#define BLK_TC_HARDBARRIER	BLK_TC_BARRIER
 #define BLK_TC_RAHEAD		BLK_TC_AHEAD
 
 /* The ilog2() calls fall out because they're constant */
@@ -196,7 +195,6 @@ static void __blk_add_trace(struct blk_t
 		return;
 
 	what |= ddir_act[rw & WRITE];
-	what |= MASK_TC_BIT(rw, HARDBARRIER);
 	what |= MASK_TC_BIT(rw, SYNC);
 	what |= MASK_TC_BIT(rw, RAHEAD);
 	what |= MASK_TC_BIT(rw, META);
@@ -1807,8 +1805,6 @@ void blk_fill_rwbs(char *rwbs, u32 rw, i
 
 	if (rw & REQ_RAHEAD)
 		rwbs[i++] = 'A';
-	if (rw & REQ_HARDBARRIER)
-		rwbs[i++] = 'B';
 	if (rw & REQ_SYNC)
 		rwbs[i++] = 'S';
 	if (rw & REQ_META)
Acked-by on v1 1/1
#46 lore raw

Re: [PATCH] block: remove REQ_HARDBARRIER

Quoted 2 lines · Christoph Hellwig

- setting of ordered tags in uas - dead code copied from old scsi drivers.

Oops. Meant to take that out. Thanks.

Signature · 4 lines
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
#47 lore raw

Re: [PATCH] block: remove REQ_HARDBARRIER

In reply to Christoph Hellwig
Quoted 14 lines · Christoph Hellwig

REQ_HARDBARRIER is dead now, so remove the leftovers. What's left at this point is:

- various checks inside the block layer.
- sanity checks in bio based drivers.
- now unused bio_empty_barrier helper.
- Xen blockfront use of BLKIF_OP_WRITE_BARRIER - it's dead for a while, but Xen really needs to sort out it's barrier situaton.
- setting of ordered tags in uas - dead code copied from old scsi drivers.
- scsi different retry for barriers - it's dead and should have been removed when flushes were converted to FS requests.
- blktrace handling of barriers - removed. Someone who knows blktrace better should add support for REQ_FLUSH and REQ_FUA, though.

Thanks Christoph, applied for 2.6.37 (but with a few days of brewing).

Signature · 1 line
Jens Axboe
#48 lore raw

Re: [PATCH] block: remove REQ_HARDBARRIER

Quoted 1 line · Jens Axboe

Thanks Christoph, applied for 2.6.37 (but with a few days of brewing).

Btw, in case it wasn't clear - this needs to be applied only after the drbd patches that remove the drbd use of REQ_HARDBARRIER.

#49 lore raw

Re: [PATCH] block: remove REQ_HARDBARRIER

In reply to Jens Axboe
Quoted 1 line · Jens Axboe

Thanks Christoph, applied for 2.6.37 (but with a few days of brewing).

So, what's the plan? I'd really hate to see 2.6.37 getting released with those stale barrier bits left in.

#50 lore raw

Re: [PATCH] block: remove REQ_HARDBARRIER

Quoted 5 lines · Christoph Hellwig
Quoted 1 line · Jens Axboe

Thanks Christoph, applied for 2.6.37 (but with a few days of brewing).

So, what's the plan? I'd really hate to see 2.6.37 getting released with those stale barrier bits left in.

It's queued up, I'll be pushing this out this week:

git://git.kernel.dk/linux-2.6-block.git for-linus

Christoph Hellwig (1):
block: remove REQ_HARDBARRIER

Daniel J Blueman (1):
ioprio: fix RCU locking around task dereference

Jens Axboe (7):
Merge branch 'for-jens' of git://git.drbd.org/linux-2.6-drbd into for-2.6.37/drivers
block: check for proper length of iov entries in blk_rq_map_user_iov()
block: take care not to overflow when calculating total iov length
block: limit vec count in bio_kmalloc() and bio_alloc_map_data()
bio: take care not overflow page count when mapping/copying user data
cciss: fix proc warning on attempt to remove non-existant directory Merge branch 'for-2.6.37/drivers' into for-linus

Lars Ellenberg (6):
drbd: consolidate explicit drbd_md_sync into drbd_create_new_uuid
drbd: tag a few error messages with "assert failed"
drbd: fix potential deadlock on detach
drbd: fix potential data divergence after multiple failures
drbd: fix a misleading printk
drbd: rate limit an error message

Mike Snitzer (1):
block: read i_size with i_size_read()

Philipp Reisner (4):
drbd: Silenced an assert
drbd: Removed the BIO_RW_BARRIER support form the receiver/epoch code
drbd: REQ_HARDBARRIER -> REQ_FUA transition for meta data accesses
drbd: Removed checks for REQ_HARDBARRIER on incomming BIOs

Sergey Senozhatsky (1):
ioprio: rcu_read_lock/unlock protect find_task_by_vpid call (V2)

Stephen M. Cameron (5):
cciss: fix board status waiting code
cciss: Use kernel provided PCI state save and restore functions
cciss: limit commands allocated on reset_devices
cciss: use usleep_range not msleep for small sleeps
cciss: remove controllers supported by hpsa

Vasiliy Kulikov (1):
block: ioctl: fix information leak to userland

 block/blk-core.c                   |   11 +--
 block/blk-map.c                    |    2 +
 block/compat_ioctl.c               |    4 +-
 block/elevator.c                   |    4 +-
 block/ioctl.c                      |    7 +-
 block/scsi_ioctl.c                 |   34 ++++--
 drivers/block/aoe/aoeblk.c         |    3 -
 drivers/block/cciss.c              |  131 ++++++++++------------
 drivers/block/cciss.h              |    4 +
 drivers/block/drbd/drbd_actlog.c   |   42 ++++---
 drivers/block/drbd/drbd_int.h      |   52 ++++-----
 drivers/block/drbd/drbd_main.c     |  148 ++++++++++++++-----------
 drivers/block/drbd/drbd_nl.c       |   25 +++-
 drivers/block/drbd/drbd_proc.c     |    1 -
 drivers/block/drbd/drbd_receiver.c |  217 ++++++------------------------------
 drivers/block/drbd/drbd_req.c      |   38 +++----
 drivers/block/drbd/drbd_worker.c   |   23 +----
 drivers/block/loop.c               |    6 -
 drivers/block/xen-blkfront.c       |    2 -
 drivers/md/md.c                    |   20 ++--
 drivers/scsi/scsi_error.c          |   18 +--
 drivers/usb/storage/uas.c          |    5 +-
 fs/bio.c                           |   23 ++++-
 fs/ioprio.c                        |   18 +++-
 include/linux/bio.h                |    4 -
 include/linux/blk_types.h          |    6 +-
 include/linux/blkdev.h             |    3 +-
 include/linux/drbd.h               |    2 +-
 kernel/trace/blktrace.c            |    4 -
 29 files changed, 360 insertions(+), 497 deletions(-)
Signature · 1 line
Jens Axboe
#51 lore raw

Re: [PATCH] block: remove REQ_HARDBARRIER

In reply to Christoph Hellwig
Quoted 2 lines · Christoph Hellwig

- blktrace handling of barriers - removed. Someone who knows blktrace better should add support for REQ_FLUSH and REQ_FUA, though.

I'll update blktrace to be current there, btw.

Signature · 1 line
Jens Axboe
author Reviewed-by on v1 1/1
#52 lore raw

Re: [PATCH] block: remove REQ_HARDBARRIER

In reply to Christoph Hellwig
Quoted 15 lines · Christoph Hellwig

REQ_HARDBARRIER is dead now, so remove the leftovers. What's left at this point is:

- various checks inside the block layer.
- sanity checks in bio based drivers.
- now unused bio_empty_barrier helper.
- Xen blockfront use of BLKIF_OP_WRITE_BARRIER - it's dead for a while, but Xen really needs to sort out it's barrier situaton.
- setting of ordered tags in uas - dead code copied from old scsi drivers.
- scsi different retry for barriers - it's dead and should have been removed when flushes were converted to FS requests.
- blktrace handling of barriers - removed. Someone who knows blktrace better should add support for REQ_FLUSH and REQ_FUA, though.

Quoted 1 line

Looks good to me.

Thanks.

Signature · 1 line
tejun