[PATCH] block: BARRIER request should imply SYNC

[PATCH] block: BARRIER request should imply SYNC

[Christoph Hellwig]

A barrier request should by defintion have priority in get_request
and let the queue be unplugged immediately as it's blocking all forward
progress due to the queue draining.

Most filesystems already get this implicitly by the way how submit_bh
treats the buffer_ordered flag, and gfs2 sets it explicitly.  But btrfs
and XFS are still forgetting to set the flag, as is blkdev_issue_flush
and some places in DM/MD.

For XFS on metadata heavy workloads this gives a consistent speedup
in the 2-3% range.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2010-06-17 08:37:05.238025121 +0200
+++ linux-2.6/include/linux/fs.h	2010-06-17 08:37:27.967253715 +0200
@@ -136,7 +136,7 @@ struct inodes_stat_t {
  * SWRITE_SYNC
  * SWRITE_SYNC_PLUG	Like WRITE_SYNC/WRITE_SYNC_PLUG, but locks the buffer.
  *			See SWRITE.
- * WRITE_BARRIER	Like WRITE, but tells the block layer that all
+ * WRITE_BARRIER	Like WRITE_SYNC, but tells the block layer that all
  *			previously submitted writes must be safely on storage
  *			before this one is started. Also guarantees that when
  *			this write is complete, it itself is also safely on
@@ -159,7 +159,7 @@ struct inodes_stat_t {
 #define SWRITE_SYNC_PLUG	\
 			(SWRITE | (1 << BIO_RW_SYNCIO) | (1 << BIO_RW_NOIDLE))
 #define SWRITE_SYNC	(SWRITE_SYNC_PLUG | (1 << BIO_RW_UNPLUG))
-#define WRITE_BARRIER	(WRITE | (1 << BIO_RW_BARRIER))
+#define WRITE_BARRIER	(WRITE_SYNC | (1 << BIO_RW_BARRIER))
 
 /*
  * These aren't really reads or writes, they pass down information about
Index: linux-2.6/fs/gfs2/log.c
===================================================================
--- linux-2.6.orig/fs/gfs2/log.c	2010-06-17 08:42:18.020253855 +0200
+++ linux-2.6/fs/gfs2/log.c	2010-06-17 08:42:54.135032427 +0200
@@ -595,7 +595,7 @@ static void log_write_header(struct gfs2
 	if (test_bit(SDF_NOBARRIERS, &sdp->sd_flags))
 		goto skip_barrier;
 	get_bh(bh);
-	submit_bh(WRITE_SYNC | (1 << BIO_RW_BARRIER) | (1 << BIO_RW_META), bh);
+	submit_bh(WRITE_BARRIER | (1 << BIO_RW_META), bh);
 	wait_on_buffer(bh);
 	if (buffer_eopnotsupp(bh)) {
 		clear_buffer_eopnotsupp(bh);

Re: [PATCH] block: BARRIER request should imply SYNC

[Jens Axboe]

On 2010-06-17 08:54, Christoph Hellwig wrote:
> 
> A barrier request should by defintion have priority in get_request
> and let the queue be unplugged immediately as it's blocking all forward
> progress due to the queue draining.
> 
> Most filesystems already get this implicitly by the way how submit_bh
> treats the buffer_ordered flag, and gfs2 sets it explicitly.  But btrfs
> and XFS are still forgetting to set the flag, as is blkdev_issue_flush
> and some places in DM/MD.
> 
> For XFS on metadata heavy workloads this gives a consistent speedup
> in the 2-3% range.

Thanks Christoph, applied.

-- 
Jens Axboe