Re: [PATCH] ext4: Set barrier=0 when block device does not advertise flush support

Previous thread: linux-next: manual merge of the net tree with the wireless-current tree by Stephen Rothwell on Thursday, December 2, 2010 - 4:45 pm. (1 message)

Next thread: [PATCH 2/6] mm: add FOLL_MLOCK follow_page flag. by Michel Lespinasse on Thursday, December 2, 2010 - 5:16 pm. (2 messages)
From: Darrick J. Wong
Date: Thursday, December 2, 2010 - 5:16 pm

If the user tries to enable write flushes with "barrier=1" and the underlying
block device does not support flushes, print a message and set barrier=0.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 fs/ext4/super.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e32195d..098dcf0 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3124,6 +3124,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 			   &journal_ioprio, NULL, 0))
 		goto failed_mount;
 
+	/* Catch barrier=1 and no flush support */
+	if (test_opt(sb, BARRIER) &&
+		     !(sb->s_bdev->bd_disk->queue->flush_flags & REQ_FLUSH)) {
+		ext4_msg(sb, KERN_WARNING, "flush not supported; disabling.");
+		clear_opt(sbi->s_mount_opt, BARRIER);
+	}
+
 	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
 		(test_opt(sb, POSIX_ACL) ? MS_POSIXACL : 0);
 
@@ -4198,6 +4205,13 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 		goto restore_opts;
 	}
 
+	/* Catch barrier=1 and no flush support */
+	if (test_opt(sb, BARRIER) &&
+		     !(sb->s_bdev->bd_disk->queue->flush_flags & REQ_FLUSH)) {
+		ext4_msg(sb, KERN_WARNING, "flush not supported; disabling.");
+		clear_opt(sbi->s_mount_opt, BARRIER);
+	}
+
 	if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
 		ext4_abort(sb, "Abort forced by user");
 
--

From: Darrick J. Wong
Date: Friday, December 3, 2010 - 2:14 am

Are we ready to remove the barrier= mount option at this point?  How many users
exist who use barrier=0 to speed up write performance when they're willing to
take on the added safety risk?

I've noticed that provisioning goes faster if one mounts the filesystem with
barrier=0; so long as the control software turns barriers on after the deploy
finishes and always restarts the deploy after a failure, a power failure on the
client system won't cause problems.  Unfortunately, there doesn't seem to be
any other way to communicate that relaxation to the code.

Personally I'd rather the knob remain in ext4 on the grounds that I know my
workloads and can judge the appropriate level of risk, especially since ext4
picks the safe option by default.  However, I'd prefer /proc/mounts not
misrepresent the status of flush support, to the best of ext4's knowledge.

--D
--

From: Christoph Hellwig
Date: Monday, December 6, 2010 - 6:39 am

barrier=0 really means losemydata=1.  The plan I discussed with Jens was
to allow to disable the flush and fua semantics in the block layer, so
we'll have one new tunable for that, which is documented to causes these

That's bullshit.  The barrier option has traditionally meant that we
sent barrier requests, and now means thatwe send flush+fua requests.
There's no reason for a warning and option mislabling just because you
got the most efficient implementation of it.

--

From: Darrick J. Wong
Date: Monday, December 6, 2010 - 11:09 am

Oh.  I wasn't aware that anyone was planning to put in a tuning knob for
flush/fua, red warning light or otherwise.  What is the name of the tunable,

Well then, let's remove the barrier= mount flag altogether.  No need for strong
language over a minor issue. :)  When I see some patches I will push this
through my testing setup and report back what data I collect.

--

Previous thread: linux-next: manual merge of the net tree with the wireless-current tree by Stephen Rothwell on Thursday, December 2, 2010 - 4:45 pm. (1 message)

Next thread: [PATCH 2/6] mm: add FOLL_MLOCK follow_page flag. by Michel Lespinasse on Thursday, December 2, 2010 - 5:16 pm. (2 messages)