[PATCHSET] Refactor barrier=/nobarrier flags from fs to block layer

[PATCHSET] Refactor barrier=/nobarrier flags from fs to block layer

[Darrick J. Wong]

Hello,

>From what I can tell, most of the filesystems that know how to issue commands
to flush the write cache also have some mechanism for the user to override
whether or not the filesystem actually issues those flushes.  Unfortunately,
the term "barrier" is obsolete having been changed into flushes in 2.6.36, and
many of the filesystems implement the mount options with slightly different
syntaxes (barrier=[0|1|none|flush], nobarrier, etc).

This patchset adds to the block layer a sysfs knob that an administrator can
use to disable flushes, and removes the mount options from the filesystem code.
As a starting point, I'm removing the mount options and flush toggle from
jbd2/ext4.

Anyway, I'm looking for some feedback about refactoring the barrier/flush
control knob into the block layer.  It sounds like we want a knob that picks
the safest option (issue flushes when supported) unless the administrator
decides that it is appropriate to do otherwise.  I suspect that there are good
arguments for not having a knob at all, and good arguments for a safe knob.
However, since I don't see the barrier options being removed en masse, I'm
assuming that we still want a knob somewhere.  Do we need the ignore_fua knob
too?  Is this the proper way to deprecate mount options out of filesystems?

--D

[PATCH 1/3] block: Create sysfs knobs to override FLUSH/FUA support flags

[Darrick J. Wong]

This patch is the first in a series to refactor the barrier= mount options out
of the filesystem code.  This patch adds sysfs knobs to disable flush and FUA;
of course, the automatic default is the safe choice, i.e. to leave them
enabled.  Obviously, only a seasoned administrator should ever be overriding
the defaults.

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

 block/blk-settings.c   |    1 +
 block/blk-sysfs.c      |   66 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h |    1 +
 3 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 36c8c1f..719c990 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -802,6 +802,7 @@ void blk_queue_flush(struct request_queue *q, unsigned int flush)
 		flush &= ~REQ_FUA;
 
 	q->flush_flags = flush & (REQ_FLUSH | REQ_FUA);
+	q->hw_flush_flags = q->flush_flags;
 }
 EXPORT_SYMBOL_GPL(blk_queue_flush);
 
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 41fb691..af872a8 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -31,6 +31,58 @@ queue_var_store(unsigned long *var, const char *page, size_t count)
 	return count;
 }
 
+static ssize_t queue_flushflag_ignore_show(struct request_queue *q, char *page,
+					   unsigned int which)
+{
+	if (!(q->hw_flush_flags & which))
+		return sprintf(page, "0\n");
+
+	if (!(q->flush_flags & which))
+		return sprintf(page, "1\n");
+
+	return sprintf(page, "0\n");
+}
+
+static ssize_t
+queue_flushflag_ignore_store(struct request_queue *q, const char *page,
+			     size_t count, unsigned int which)
+{
+	unsigned long ignore;
+	ssize_t ret = queue_var_store(&ignore, page, count);
+
+	if (!(q->hw_flush_flags & which))
+		return -EINVAL;
+
+	if (ignore)
+		q->flush_flags &= ~which;
+	else
+		q->flush_flags |= which;
+
+	return ret;
+}
+
+static ssize_t queue_ignore_flush_show(struct request_queue *q, char *page)
+{
+	return queue_flushflag_ignore_show(q, page, REQ_FLUSH);
+}
+
+static ssize_t queue_ignore_flush_store(struct request_queue *q,
+					const char *page, size_t count)
+{
+	return queue_flushflag_ignore_store(q, page, count, REQ_FLUSH);
+}
+
+static ssize_t queue_ignore_fua_show(struct request_queue *q, char *page)
+{
+	return queue_flushflag_ignore_show(q, page, REQ_FUA);
+}
+
+static ssize_t queue_ignore_fua_store(struct request_queue *q,
+				      const char *page, size_t count)
+{
+	return queue_flushflag_ignore_store(q, page, count, REQ_FUA);
+}
+
 static ssize_t queue_requests_show(struct request_queue *q, char *page)
 {
 	return queue_var_show(q->nr_requests, (page));
@@ -265,6 +317,18 @@ queue_rq_affinity_store(struct request_queue *q, const char *page, size_t count)
 	return ret;
 }
 
+static struct queue_sysfs_entry queue_ignore_flush_entry = {
+	.attr = {.name = "ignore_flush", .mode = S_IRUGO | S_IWUSR },
+	.show = queue_ignore_flush_show,
+	.store = queue_ignore_flush_store,
+};
+
+static struct queue_sysfs_entry queue_ignore_fua_entry = {
+	.attr = {.name = "ignore_fua", .mode = S_IRUGO | S_IWUSR },
+	.show = queue_ignore_fua_show,
+	.store = queue_ignore_fua_store,
+};
+
 static struct queue_sysfs_entry queue_requests_entry = {
 	.attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
 	.show = queue_requests_show,
@@ -380,6 +444,8 @@ static struct queue_sysfs_entry queue_random_entry = {
 };
 
 static struct attribute *default_attrs[] = {
+	&queue_ignore_flush_entry.attr,
+	&queue_ignore_fua_entry.attr,
 	&queue_requests_entry.attr,
 	&queue_ra_entry.attr,
 	&queue_max_hw_sectors_entry.attr,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8a082a5..daa4e6b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -368,6 +368,7 @@ struct request_queue
 	 * for flush operations
 	 */
 	unsigned int		flush_flags;
+	unsigned int		hw_flush_flags;
 	unsigned int		flush_pending_idx:1;
 	unsigned int		flush_running_idx:1;
 	unsigned long		flush_pending_since;

[PATCH 2/3] jbd2: Remove barrier feature conditional flag (or: always issue flushes)

As part of migrating the FLUSH/FUA knob to the block layer, remove the journal
flags and various conditionals in jbd2 that surround flush issue calls in favor
of always issuing the flush.  The block layer will handle gracefully the
situation where a FLUSH or FUA request is issued to a device that doesn't
support it.

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

 fs/ext4/fsync.c      |    5 ++---
 fs/ext4/super.c      |    7 -------
 fs/jbd2/checkpoint.c |    3 +--
 fs/jbd2/commit.c     |   10 +++-------
 fs/ocfs2/journal.c   |    4 ----
 include/linux/jbd2.h |    1 -
 6 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
index 7829b28..8c99359 100644
--- a/fs/ext4/fsync.c
+++ b/fs/ext4/fsync.c
@@ -208,12 +208,11 @@ int ext4_sync_file(struct file *file, int datasync)
 		 * the journal device.)
 		 */
 		if (ext4_should_writeback_data(inode) &&
-		    (journal->j_fs_dev != journal->j_dev) &&
-		    (journal->j_flags & JBD2_BARRIER))
+		    (journal->j_fs_dev != journal->j_dev))
 			blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL,
 					NULL);
 		ret = jbd2_log_wait_commit(journal, commit_tid);
-	} else if (journal->j_flags & JBD2_BARRIER)
+	} else
 		blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
 	return ret;
 }
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 48ce561..7379829 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3704,10 +3704,6 @@ static void ext4_init_journal_params(struct super_block *sb, journal_t *journal)
 	journal->j_max_batch_time = sbi->s_max_batch_time;
 
 	write_lock(&journal->j_state_lock);
-	if (test_opt(sb, BARRIER))
-		journal->j_flags |= JBD2_BARRIER;
-	else
-		journal->j_flags &= ~JBD2_BARRIER;
 	if (test_opt(sb, DATA_ERR_ABORT))
 		journal->j_flags |= JBD2_ABORT_ON_SYNCDATA_ERR;
 	else
@@ -3899,9 +3895,6 @@ static int ext4_load_journal(struct super_block *sb,
 			return -EINVAL;
 	}
 
-	if (!(journal->j_flags & JBD2_BARRIER))
-		ext4_msg(sb, KERN_INFO, "barriers disabled");
-
 	if (!really_read_only && test_opt(sb, UPDATE_JOURNAL)) {
 		err = jbd2_journal_update_format(journal);
 		if (err)  {
diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 6a79fd0..02eb6b9 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -540,8 +540,7 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
 	 * correctness.  Fortunately jbd2_cleanup_journal_tail()
 	 * doesn't get called all that often.
 	 */
-	if ((journal->j_fs_dev != journal->j_dev) &&
-	    (journal->j_flags & JBD2_BARRIER))
+	if (journal->j_fs_dev != journal->j_dev)
 		blkdev_issue_flush(journal->j_fs_dev, GFP_KERNEL, NULL);
 	if (!(journal->j_flags & JBD2_ABORT))
 		jbd2_journal_update_superblock(journal, 1);
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index f3ad159..e90ec0d 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -134,8 +134,7 @@ static int journal_submit_commit_record(journal_t *journal,
 	set_buffer_uptodate(bh);
 	bh->b_end_io = journal_end_buffer_io_sync;
 
-	if (journal->j_flags & JBD2_BARRIER &&
-	    !JBD2_HAS_INCOMPAT_FEATURE(journal,
+	if (!JBD2_HAS_INCOMPAT_FEATURE(journal,
 				       JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT))
 		ret = submit_bh(WRITE_SYNC_PLUG | WRITE_FLUSH_FUA, bh);
 	else
@@ -686,8 +685,7 @@ start_journal_io:
 	 * the commit record
 	 */
 	if (commit_transaction->t_flushed_data_blocks &&
-	    (journal->j_fs_dev != journal->j_dev) &&
-	    (journal->j_flags & JBD2_BARRIER))
+	    (journal->j_fs_dev != journal->j_dev))
 		blkdev_issue_flush(journal->j_fs_dev, GFP_KERNEL, NULL);
 
 	/* Done it all: now write the commit record asynchronously. */
@@ -811,10 +809,8 @@ wait_for_iobuf:
 	if (!err && !is_journal_aborted(journal))
 		err = journal_wait_on_commit_record(journal, cbh);
 	if (JBD2_HAS_INCOMPAT_FEATURE(journal,
-				      JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT) &&
-	    journal->j_flags & JBD2_BARRIER) {
+				      JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT))
 		blkdev_issue_flush(journal->j_dev, GFP_KERNEL, NULL);
-	}
 
 	if (err)
 		jbd2_journal_abort(journal, err);
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index faa2303..caacabe 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -758,10 +758,6 @@ void ocfs2_set_journal_params(struct ocfs2_super *osb)
 
 	write_lock(&journal->j_state_lock);
 	journal->j_commit_interval = commit_interval;
-	if (osb->s_mount_opt & OCFS2_MOUNT_BARRIER)
-		journal->j_flags |= JBD2_BARRIER;
-	else
-		journal->j_flags &= ~JBD2_BARRIER;
 	write_unlock(&journal->j_state_lock);
 }
 
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 27e79c2..2c420ce 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -987,7 +987,6 @@ struct journal_s
 #define JBD2_ACK_ERR	0x004	/* The errno in the sb has been acked */
 #define JBD2_FLUSHED	0x008	/* The journal superblock has been flushed */
 #define JBD2_LOADED	0x010	/* The journal superblock has been loaded */
-#define JBD2_BARRIER	0x020	/* Use IDE barriers */
 #define JBD2_ABORT_ON_SYNCDATA_ERR	0x040	/* Abort the journal on file
 						 * data write error in ordered
 						 * mode */

[Ocfs2-devel] [PATCH 2/3] jbd2: Remove barrier feature conditional flag (or: always issue flushes)

[Darrick J. Wong]

As part of migrating the FLUSH/FUA knob to the block layer, remove the journal
flags and various conditionals in jbd2 that surround flush issue calls in favor
of always issuing the flush.  The block layer will handle gracefully the
situation where a FLUSH or FUA request is issued to a device that doesn't
support it.

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

 fs/ext4/fsync.c      |    5 ++---
 fs/ext4/super.c      |    7 -------
 fs/jbd2/checkpoint.c |    3 +--
 fs/jbd2/commit.c     |   10 +++-------
 fs/ocfs2/journal.c   |    4 ----
 include/linux/jbd2.h |    1 -
 6 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
index 7829b28..8c99359 100644
--- a/fs/ext4/fsync.c
+++ b/fs/ext4/fsync.c
@@ -208,12 +208,11 @@ int ext4_sync_file(struct file *file, int datasync)
 		 * the journal device.)
 		 */
 		if (ext4_should_writeback_data(inode) &&
-		    (journal->j_fs_dev != journal->j_dev) &&
-		    (journal->j_flags & JBD2_BARRIER))
+		    (journal->j_fs_dev != journal->j_dev))
 			blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL,
 					NULL);
 		ret = jbd2_log_wait_commit(journal, commit_tid);
-	} else if (journal->j_flags & JBD2_BARRIER)
+	} else
 		blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
 	return ret;
 }
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 48ce561..7379829 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3704,10 +3704,6 @@ static void ext4_init_journal_params(struct super_block *sb, journal_t *journal)
 	journal->j_max_batch_time = sbi->s_max_batch_time;
 
 	write_lock(&journal->j_state_lock);
-	if (test_opt(sb, BARRIER))
-		journal->j_flags |= JBD2_BARRIER;
-	else
-		journal->j_flags &= ~JBD2_BARRIER;
 	if (test_opt(sb, DATA_ERR_ABORT))
 		journal->j_flags |= JBD2_ABORT_ON_SYNCDATA_ERR;
 	else
@@ -3899,9 +3895,6 @@ static int ext4_load_journal(struct super_block *sb,
 			return -EINVAL;
 	}
 
-	if (!(journal->j_flags & JBD2_BARRIER))
-		ext4_msg(sb, KERN_INFO, "barriers disabled");
-
 	if (!really_read_only && test_opt(sb, UPDATE_JOURNAL)) {
 		err = jbd2_journal_update_format(journal);
 		if (err)  {
diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 6a79fd0..02eb6b9 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -540,8 +540,7 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
 	 * correctness.  Fortunately jbd2_cleanup_journal_tail()
 	 * doesn't get called all that often.
 	 */
-	if ((journal->j_fs_dev != journal->j_dev) &&
-	    (journal->j_flags & JBD2_BARRIER))
+	if (journal->j_fs_dev != journal->j_dev)
 		blkdev_issue_flush(journal->j_fs_dev, GFP_KERNEL, NULL);
 	if (!(journal->j_flags & JBD2_ABORT))
 		jbd2_journal_update_superblock(journal, 1);
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index f3ad159..e90ec0d 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -134,8 +134,7 @@ static int journal_submit_commit_record(journal_t *journal,
 	set_buffer_uptodate(bh);
 	bh->b_end_io = journal_end_buffer_io_sync;
 
-	if (journal->j_flags & JBD2_BARRIER &&
-	    !JBD2_HAS_INCOMPAT_FEATURE(journal,
+	if (!JBD2_HAS_INCOMPAT_FEATURE(journal,
 				       JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT))
 		ret = submit_bh(WRITE_SYNC_PLUG | WRITE_FLUSH_FUA, bh);
 	else
@@ -686,8 +685,7 @@ start_journal_io:
 	 * the commit record
 	 */
 	if (commit_transaction->t_flushed_data_blocks &&
-	    (journal->j_fs_dev != journal->j_dev) &&
-	    (journal->j_flags & JBD2_BARRIER))
+	    (journal->j_fs_dev != journal->j_dev))
 		blkdev_issue_flush(journal->j_fs_dev, GFP_KERNEL, NULL);
 
 	/* Done it all: now write the commit record asynchronously. */
@@ -811,10 +809,8 @@ wait_for_iobuf:
 	if (!err && !is_journal_aborted(journal))
 		err = journal_wait_on_commit_record(journal, cbh);
 	if (JBD2_HAS_INCOMPAT_FEATURE(journal,
-				      JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT) &&
-	    journal->j_flags & JBD2_BARRIER) {
+				      JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT))
 		blkdev_issue_flush(journal->j_dev, GFP_KERNEL, NULL);
-	}
 
 	if (err)
 		jbd2_journal_abort(journal, err);
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index faa2303..caacabe 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -758,10 +758,6 @@ void ocfs2_set_journal_params(struct ocfs2_super *osb)
 
 	write_lock(&journal->j_state_lock);
 	journal->j_commit_interval = commit_interval;
-	if (osb->s_mount_opt & OCFS2_MOUNT_BARRIER)
-		journal->j_flags |= JBD2_BARRIER;
-	else
-		journal->j_flags &= ~JBD2_BARRIER;
 	write_unlock(&journal->j_state_lock);
 }
 
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 27e79c2..2c420ce 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -987,7 +987,6 @@ struct journal_s
 #define JBD2_ACK_ERR	0x004	/* The errno in the sb has been acked */
 #define JBD2_FLUSHED	0x008	/* The journal superblock has been flushed */
 #define JBD2_LOADED	0x010	/* The journal superblock has been loaded */
-#define JBD2_BARRIER	0x020	/* Use IDE barriers */
 #define JBD2_ABORT_ON_SYNCDATA_ERR	0x040	/* Abort the journal on file
 						 * data write error in ordered
 						 * mode */

[PATCH 3/3] ext4: Deprecate barrier= and nobarrier mount options

[Darrick J. Wong]

As part of migrating the FLUSH/FUA knob to the block layer, remove the support
code for the barrier-related mount options and remove the conditionals around
flushes in favor of always issuing the flush.  The block layer will handle
gracefully the situation where a FLUSH or FUA request is issued to a device
that doesn't support it.  Modify the option parsing code to print a warning if
someone tries to use the old mount option.

Note: The nobarrier bit in the default mount flags is now useless.

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

 fs/ext4/ext4.h  |    1 -
 fs/ext4/super.c |   23 ++---------------------
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 0c8d97b..fa4c688 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -893,7 +893,6 @@ struct ext4_inode_info {
 #define EXT4_MOUNT_XATTR_USER		0x04000	/* Extended user attributes */
 #define EXT4_MOUNT_POSIX_ACL		0x08000	/* POSIX Access Control Lists */
 #define EXT4_MOUNT_NO_AUTO_DA_ALLOC	0x10000	/* No auto delalloc mapping */
-#define EXT4_MOUNT_BARRIER		0x20000 /* Use block barriers */
 #define EXT4_MOUNT_QUOTA		0x80000 /* Some quota option set */
 #define EXT4_MOUNT_USRQUOTA		0x100000 /* "old" user quota */
 #define EXT4_MOUNT_GRPQUOTA		0x200000 /* "old" group quota */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 7379829..cd8e7b8 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1022,13 +1022,6 @@ static int ext4_show_options(struct seq_file *seq, struct vfsmount *vfs)
 			   (unsigned) sbi->s_min_batch_time);
 	}
 
-	/*
-	 * We're changing the default of barrier mount option, so
-	 * let's always display its mount state so it's clear what its
-	 * status is.
-	 */
-	seq_puts(seq, ",barrier=");
-	seq_puts(seq, test_opt(sb, BARRIER) ? "1" : "0");
 	if (test_opt(sb, JOURNAL_ASYNC_COMMIT))
 		seq_puts(seq, ",journal_async_commit");
 	else if (test_opt(sb, JOURNAL_CHECKSUM))
@@ -1701,18 +1694,9 @@ set_qf_format:
 			sbi->s_mount_flags |= EXT4_MF_FS_ABORTED;
 			break;
 		case Opt_nobarrier:
-			clear_opt(sb, BARRIER);
-			break;
 		case Opt_barrier:
-			if (args[0].from) {
-				if (match_int(&args[0], &option))
-					return 0;
-			} else
-				option = 1;	/* No argument, default to 1 */
-			if (option)
-				set_opt(sb, BARRIER);
-			else
-				clear_opt(sb, BARRIER);
+			ext4_msg(sb, KERN_INFO, "barrier options are "
+				 "deprecated.  Use ignore_flush in sysfs.");
 			break;
 		case Opt_ignore:
 			break;
@@ -3125,9 +3109,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_min_batch_time = EXT4_DEF_MIN_BATCH_TIME;
 	sbi->s_max_batch_time = EXT4_DEF_MAX_BATCH_TIME;
 
-	if ((def_mount_opts & EXT4_DEFM_NOBARRIER) == 0)
-		set_opt(sb, BARRIER);
-
 	/*
 	 * enable delayed allocation by default
 	 * Use -o nodelalloc to turn it off

Re: [PATCH 1/3] block: Create sysfs knobs to override FLUSH/FUA support flags

[Tejun Heo]

On Tue, Jan 25, 2011 at 11:16:26PM -0800, Darrick J. Wong wrote:
> This patch is the first in a series to refactor the barrier= mount options out
> of the filesystem code.  This patch adds sysfs knobs to disable flush and FUA;
> of course, the automatic default is the safe choice, i.e. to leave them
> enabled.  Obviously, only a seasoned administrator should ever be overriding
> the defaults.

Hmmm... wouldn't it be better to just export flush and fua instead of
ignore_*?  So that the admin can turn things on and off as [s]he seems
fit?  Also, it might be better to export them in a single attribute,
say cache_control or something.  Only subset of the combinations make
sense anyway - none, flush, flush_fua.

Thanks.

-- 
tejun

Re: [PATCH 3/3] ext4: Deprecate barrier= and nobarrier mount options

[Tejun Heo]

Hello,

On Tue, Jan 25, 2011 at 11:23:29PM -0800, Darrick J. Wong wrote:
> As part of migrating the FLUSH/FUA knob to the block layer, remove the support
> code for the barrier-related mount options and remove the conditionals around
> flushes in favor of always issuing the flush.  The block layer will handle
> gracefully the situation where a FLUSH or FUA request is issued to a device
> that doesn't support it.  Modify the option parsing code to print a warning if
> someone tries to use the old mount option.
> 
> Note: The nobarrier bit in the default mount flags is now useless.

The option is something which users are already quite familiar with.
I think we'll just have to carry this around.  What we can do, tho, is
moving the actual control mechanism to block layer -
ie. blkdev_skip_flush() or something like that which ignores flush
requests for the current exclusive opener.

Thanks.

-- 
tejun

Re: [PATCH 3/3] ext4: Deprecate barrier= and nobarrier mount options

[Jan Kara]

  Hello,

On Wed 26-01-11 10:36:32, Tejun Heo wrote:
> On Tue, Jan 25, 2011 at 11:23:29PM -0800, Darrick J. Wong wrote:
> > As part of migrating the FLUSH/FUA knob to the block layer, remove the support
> > code for the barrier-related mount options and remove the conditionals around
> > flushes in favor of always issuing the flush.  The block layer will handle
> > gracefully the situation where a FLUSH or FUA request is issued to a device
> > that doesn't support it.  Modify the option parsing code to print a warning if
> > someone tries to use the old mount option.
> > 
> > Note: The nobarrier bit in the default mount flags is now useless.
> 
> The option is something which users are already quite familiar with.
> I think we'll just have to carry this around.  What we can do, tho, is
> moving the actual control mechanism to block layer -
> ie. blkdev_skip_flush() or something like that which ignores flush
> requests for the current exclusive opener.
  Ted should have a final word about this but I believe it's possible to
deprecate the mount options. Maybe with some transition period where
deprecation message is shown but the option actually still works. That
being said I'm not sure what we should do when someone has a disk with two
partitions and one partition is mounted with barriers and another one
without them - sure, one has to think hard to find a sane use case for this
(possibly if user does not care about data after a crash on one of the
partitions, in which case he should probably use nojournal mode) but it
should probably work.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 3/3] ext4: Deprecate barrier= and nobarrier mount options

[Tejun Heo]

Hello,

On Wed, Jan 26, 2011 at 11:47:34AM +0100, Jan Kara wrote:
>   Ted should have a final word about this but I believe it's possible to
> deprecate the mount options. Maybe with some transition period where
> deprecation message is shown but the option actually still works. That
> being said I'm not sure what we should do when someone has a disk with two
> partitions and one partition is mounted with barriers and another one
> without them - sure, one has to think hard to find a sane use case for this
> (possibly if user does not care about data after a crash on one of the
> partitions, in which case he should probably use nojournal mode) but it
> should probably work.

The policy can be made per-bdev (which maps to per-partition), so I
don't think that's a big problem.

Thanks.

-- 
tejun

Re: [PATCHSET] Refactor barrier=/nobarrier flags from fs to block layer

[Ric Wheeler]

On 01/26/2011 02:12 AM, Darrick J. Wong wrote:
> Hello,
>
>  From what I can tell, most of the filesystems that know how to issue commands
> to flush the write cache also have some mechanism for the user to override
> whether or not the filesystem actually issues those flushes.  Unfortunately,
> the term "barrier" is obsolete having been changed into flushes in 2.6.36, and
> many of the filesystems implement the mount options with slightly different
> syntaxes (barrier=[0|1|none|flush], nobarrier, etc).

Why remove the mount option? We have been using that term and educating users 
about how to use it for many, many years.

I see no reason to remove mount options at all.

Ric

> This patchset adds to the block layer a sysfs knob that an administrator can
> use to disable flushes, and removes the mount options from the filesystem code.
> As a starting point, I'm removing the mount options and flush toggle from
> jbd2/ext4.
>
> Anyway, I'm looking for some feedback about refactoring the barrier/flush
> control knob into the block layer.  It sounds like we want a knob that picks
> the safest option (issue flushes when supported) unless the administrator
> decides that it is appropriate to do otherwise.  I suspect that there are good
> arguments for not having a knob at all, and good arguments for a safe knob.
> However, since I don't see the barrier options being removed en masse, I'm
> assuming that we still want a knob somewhere.  Do we need the ignore_fua knob
> too?  Is this the proper way to deprecate mount options out of filesystems?
>
> --D

Re: [PATCHSET] Refactor barrier=/nobarrier flags from fs to block layer

[Ric Wheeler]

On 01/26/2011 02:12 AM, Darrick J. Wong wrote:
> Hello,
>
>  From what I can tell, most of the filesystems that know how to issue commands
> to flush the write cache also have some mechanism for the user to override
> whether or not the filesystem actually issues those flushes.  Unfortunately,
> the term "barrier" is obsolete having been changed into flushes in 2.6.36, and
> many of the filesystems implement the mount options with slightly different
> syntaxes (barrier=[0|1|none|flush], nobarrier, etc).
>
> This patchset adds to the block layer a sysfs knob that an administrator can
> use to disable flushes, and removes the mount options from the filesystem code.
> As a starting point, I'm removing the mount options and flush toggle from
> jbd2/ext4.
>
> Anyway, I'm looking for some feedback about refactoring the barrier/flush
> control knob into the block layer.  It sounds like we want a knob that picks
> the safest option (issue flushes when supported) unless the administrator
> decides that it is appropriate to do otherwise.  I suspect that there are good
> arguments for not having a knob at all, and good arguments for a safe knob.
> However, since I don't see the barrier options being removed en masse, I'm
> assuming that we still want a knob somewhere.  Do we need the ignore_fua knob
> too?  Is this the proper way to deprecate mount options out of filesystems?
>
> --D

Just to be clear, I strongly object to remove the mount options.

"Barrier" and "poke a control knob in the block" layer are both equally 
mysterious and meaningless to real users, so I do not see this as a gain,

Ric

Re: [PATCH 3/3] ext4: Deprecate barrier= and nobarrier mount options

[Ric Wheeler]

On 01/26/2011 05:47 AM, Jan Kara wrote:
>    Hello,
>
> On Wed 26-01-11 10:36:32, Tejun Heo wrote:
>> On Tue, Jan 25, 2011 at 11:23:29PM -0800, Darrick J. Wong wrote:
>>> As part of migrating the FLUSH/FUA knob to the block layer, remove the support
>>> code for the barrier-related mount options and remove the conditionals around
>>> flushes in favor of always issuing the flush.  The block layer will handle
>>> gracefully the situation where a FLUSH or FUA request is issued to a device
>>> that doesn't support it.  Modify the option parsing code to print a warning if
>>> someone tries to use the old mount option.
>>>
>>> Note: The nobarrier bit in the default mount flags is now useless.
>> The option is something which users are already quite familiar with.
>> I think we'll just have to carry this around.  What we can do, tho, is
>> moving the actual control mechanism to block layer -
>> ie. blkdev_skip_flush() or something like that which ignores flush
>> requests for the current exclusive opener.
>    Ted should have a final word about this but I believe it's possible to
> deprecate the mount options. Maybe with some transition period where
> deprecation message is shown but the option actually still works. That
> being said I'm not sure what we should do when someone has a disk with two
> partitions and one partition is mounted with barriers and another one
> without them - sure, one has to think hard to find a sane use case for this
> (possibly if user does not care about data after a crash on one of the
> partitions, in which case he should probably use nojournal mode) but it
> should probably work.
>
> 								Honza

I would also like to suggest that mount is the right time to change a per file 
system behaviour. We have well known paths (add the mount options to /etc/fstab) 
and sys admins understand it.

If we have to poke a file, what is the suggested mechanism for doing this? 
Another, new script or user space utility that needs to be run at mount time for 
each file system?

If we did want to deprecate the "barrier" name, I would leave it in place and 
replace it with something meaningful (at mount time!) to users like 
"data_safety" and "no_data_safety" modes :)

Ric

Re: [PATCH 3/3] ext4: Deprecate barrier= and nobarrier mount options

[Tejun Heo]

Hello,

On Wed, Jan 26, 2011 at 07:16:18AM -0500, Ric Wheeler wrote:
> I would also like to suggest that mount is the right time to change
> a per file system behaviour. We have well known paths (add the mount
> options to /etc/fstab) and sys admins understand it.
> 
> If we have to poke a file, what is the suggested mechanism for doing
> this? Another, new script or user space utility that needs to be run
> at mount time for each file system?
> 
> If we did want to deprecate the "barrier" name, I would leave it in
> place and replace it with something meaningful (at mount time!) to
> users like "data_safety" and "no_data_safety" modes :)

With barrier on by default these days, I don't think it's necessary to
fiddle with it too much.  It's not as important as before.  Let's
leave it as it is.  Barrier, flush, it doesn't really matter how it's
called.

In the long run tho, I think filesystems should just always enable
flush/barrier and the fiddling belongs to the block layer.

Thanks.

-- 
tejun

Re: [PATCH 3/3] ext4: Deprecate barrier= and nobarrier mount options

[torn5]

On 01/26/2011 11:47 AM, Jan Kara wrote:
>   Hello,
>
> On Wed 26-01-11 10:36:32, Tejun Heo wrote:
>   Ted should have a final word about this but I believe it's possible to
> deprecate the mount options. Maybe with some transition period where
> deprecation message is shown but the option actually still works.

+1 for this from a user.

I'd like to see this moved to blockdev sysfs in the long term.

During the transition period there could be both the old barrier mount
option (with a deprecated warning upon use) and the sysfs knobs.

Flush and FUA would be delivered to disk only if both the barriers are
enabled in the mount options (which is also the default) and the sysfs
are at "noignore" value. Alternatively, the barrier mount option could
affect the sysfs knobs values upon mount. Eventually the barrier mount
option would be removed becoming always-on, leaving only the sysfs knobs.

Re: [PATCHSET] Refactor barrier=/nobarrier flags from fs to block layer

[Eric Sandeen]

On 1/26/11 5:49 AM, Ric Wheeler wrote:
> On 01/26/2011 02:12 AM, Darrick J. Wong wrote:
>> Hello,
>>
>>  From what I can tell, most of the filesystems that know how to issue commands
>> to flush the write cache also have some mechanism for the user to override
>> whether or not the filesystem actually issues those flushes.  Unfortunately,
>> the term "barrier" is obsolete having been changed into flushes in 2.6.36, and
>> many of the filesystems implement the mount options with slightly different
>> syntaxes (barrier=[0|1|none|flush], nobarrier, etc).
>>
>> This patchset adds to the block layer a sysfs knob that an administrator can
>> use to disable flushes, and removes the mount options from the filesystem code.
>> As a starting point, I'm removing the mount options and flush toggle from
>> jbd2/ext4.
>>
>> Anyway, I'm looking for some feedback about refactoring the barrier/flush
>> control knob into the block layer.  It sounds like we want a knob that picks
>> the safest option (issue flushes when supported) unless the administrator
>> decides that it is appropriate to do otherwise.  I suspect that there are good
>> arguments for not having a knob at all, and good arguments for a safe knob.
>> However, since I don't see the barrier options being removed en masse, I'm
>> assuming that we still want a knob somewhere.  Do we need the ignore_fua knob
>> too?  Is this the proper way to deprecate mount options out of filesystems?
>>
>> --D
> 
> Just to be clear, I strongly object to remove the mount options.

Agreed, we are just finally, barely starting to win the education battle here.
Removing or changing the option now will just set us back.  It should at
LEAST remain as a deprecated option, with the deprecation message pointing
to crystal-clear documentation.

-Eric

> "Barrier" and "poke a control knob in the block" layer are both equally mysterious and meaningless to real users, so I do not see this as a gain,
> 
> Ric
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] block: Create sysfs knobs to override FLUSH/FUA support flags

[Darrick J. Wong]

On Wed, Jan 26, 2011 at 10:30:51AM +0100, Tejun Heo wrote:
> On Tue, Jan 25, 2011 at 11:16:26PM -0800, Darrick J. Wong wrote:
> > This patch is the first in a series to refactor the barrier= mount options out
> > of the filesystem code.  This patch adds sysfs knobs to disable flush and FUA;
> > of course, the automatic default is the safe choice, i.e. to leave them
> > enabled.  Obviously, only a seasoned administrator should ever be overriding
> > the defaults.
> 
> Hmmm... wouldn't it be better to just export flush and fua instead of
> ignore_*?  So that the admin can turn things on and off as [s]he seems

I considered having a general knob to override the automatic FLUSH/FUA
detection, but I thought that it wasn't a good idea to provide a mechanism to
enable features that devices don't advertise.  Mostly I was imagining horror
scenarios like USB storage devices that claim no write cache and but then catch
on fire if someone sends flush anyway.  Not using advertised features seemed
less risky.

> fit?  Also, it might be better to export them in a single attribute,
> say cache_control or something.  Only subset of the combinations make
> sense anyway - none, flush, flush_fua.

I agree.  It could be simplified even further to a simple boolean that means
"use neither" or "use whatever's supported".

--D

Re: [PATCHSET] Refactor barrier=/nobarrier flags from fs to block layer

[Darrick J. Wong]

On Wed, Jan 26, 2011 at 10:41:35AM -0600, Eric Sandeen wrote:
> On 1/26/11 5:49 AM, Ric Wheeler wrote:
> > On 01/26/2011 02:12 AM, Darrick J. Wong wrote:
> >> Hello,
> >>
> >>  From what I can tell, most of the filesystems that know how to issue commands
> >> to flush the write cache also have some mechanism for the user to override
> >> whether or not the filesystem actually issues those flushes.  Unfortunately,
> >> the term "barrier" is obsolete having been changed into flushes in 2.6.36, and
> >> many of the filesystems implement the mount options with slightly different
> >> syntaxes (barrier=[0|1|none|flush], nobarrier, etc).
> >>
> >> This patchset adds to the block layer a sysfs knob that an administrator can
> >> use to disable flushes, and removes the mount options from the filesystem code.
> >> As a starting point, I'm removing the mount options and flush toggle from
> >> jbd2/ext4.
> >>
> >> Anyway, I'm looking for some feedback about refactoring the barrier/flush
> >> control knob into the block layer.  It sounds like we want a knob that picks
> >> the safest option (issue flushes when supported) unless the administrator
> >> decides that it is appropriate to do otherwise.  I suspect that there are good
> >> arguments for not having a knob at all, and good arguments for a safe knob.
> >> However, since I don't see the barrier options being removed en masse, I'm
> >> assuming that we still want a knob somewhere.  Do we need the ignore_fua knob
> >> too?  Is this the proper way to deprecate mount options out of filesystems?
> >>
> >> --D
> > 
> > Just to be clear, I strongly object to remove the mount options.
> 
> Agreed, we are just finally, barely starting to win the education battle here.
> Removing or changing the option now will just set us back.  It should at
> LEAST remain as a deprecated option, with the deprecation message pointing
> to crystal-clear documentation.

Ok, how about a second proposal:

1. Put the sysfs knob and the toggle code in the block layer, similar to patch
#1, only make it a per-bdev toggle so each mount can have its own override
parameters.

2. Add some sort of "nocacheflush" option to the VFS layer to adjust the knob.
With this we gain a consistent mount option syntax across all the filesystems,
though what it means for a networked fs is questionable.  I guess you could
reject the mount option if there's no block device under the fs.  Also, any fs
that someday grows an issue-flush feature won't have to add its own option
parsing code.

At umount time, do we undo whatever overrides we set up at mount time?  Seems
sane to me, just wanted to run it by everyone.

3. Change the per-fs option handling code to call the same code as the VFS'
nocacheflush option.  Any fs that wants to deprecate its per-fs option handler
can do so.  Or they can stay forever.

4. Remove all the flush conditionals from the fs code in favor of letting the
block layer handle it.

Hopefully "nocacheflush" is a little more obvious.

--D

Re: [PATCHSET] Refactor barrier=/nobarrier flags from fs to block layer

[Dave Chinner]

On Wed, Jan 26, 2011 at 09:24:13AM -0800, Darrick J. Wong wrote:
> On Wed, Jan 26, 2011 at 10:41:35AM -0600, Eric Sandeen wrote:
> > On 1/26/11 5:49 AM, Ric Wheeler wrote:
> > > On 01/26/2011 02:12 AM, Darrick J. Wong wrote:
> > >> Hello,
> > >>
> > >>  From what I can tell, most of the filesystems that know how to issue commands
> > >> to flush the write cache also have some mechanism for the user to override
> > >> whether or not the filesystem actually issues those flushes.  Unfortunately,
> > >> the term "barrier" is obsolete having been changed into flushes in 2.6.36, and
> > >> many of the filesystems implement the mount options with slightly different
> > >> syntaxes (barrier=[0|1|none|flush], nobarrier, etc).
> > >>
> > >> This patchset adds to the block layer a sysfs knob that an administrator can
> > >> use to disable flushes, and removes the mount options from the filesystem code.
> > >> As a starting point, I'm removing the mount options and flush toggle from
> > >> jbd2/ext4.
> > >>
> > >> Anyway, I'm looking for some feedback about refactoring the barrier/flush
> > >> control knob into the block layer.  It sounds like we want a knob that picks
> > >> the safest option (issue flushes when supported) unless the administrator
> > >> decides that it is appropriate to do otherwise.  I suspect that there are good
> > >> arguments for not having a knob at all, and good arguments for a safe knob.
> > >> However, since I don't see the barrier options being removed en masse, I'm
> > >> assuming that we still want a knob somewhere.  Do we need the ignore_fua knob
> > >> too?  Is this the proper way to deprecate mount options out of filesystems?
> > >>
> > >> --D
> > > 
> > > Just to be clear, I strongly object to remove the mount options.
> > 
> > Agreed, we are just finally, barely starting to win the education battle here.
> > Removing or changing the option now will just set us back.  It should at
> > LEAST remain as a deprecated option, with the deprecation message pointing
> > to crystal-clear documentation.
> 
> Ok, how about a second proposal:
> 
> 1. Put the sysfs knob and the toggle code in the block layer, similar to patch
> #1, only make it a per-bdev toggle so each mount can have its own override
> parameters.

A sysfs knob just seems wrong for this. What do you do with
filesystems or block devices that span multiple block devices,
either via md, dm, mount options (XFS - separate data, log and
realtime devices) or other means (btrfs w/ multiple devices)? 

IMO, the only sane way to control this sort of behaviour is from the
top down (i.e. from the filesystem) and not from the bottom up (i.e.
from the lowest level of block devices) because the cache flushes
are only useful to the filesystem if they are consistently
implemented from the top of the storage stack to the bottom...

Also, if you allow block devices at the bottom of the stack to be
configured to ignore flushes dynamically, we need some method to
inform the upper layers that this has happened. At minimum the
filesystem needs to log the fact that their crash/power fail
consistency guarantees have changed - there's no way I'm going to
assume that users won't do something stupid if there's a knob to
tweak....

> 2. Add some sort of "nocacheflush" option to the VFS layer to adjust the knob.
> With this we gain a consistent mount option syntax across all the filesystems,
> though what it means for a networked fs is questionable.  I guess you could
> reject the mount option if there's no block device under the fs.  Also, any fs
> that someday grows an issue-flush feature won't have to add its own option
> parsing code.

We already have a relatively widely implemented mount option pair -
barrier/nobarrier is supported by ext3, ext4, btrfs, gfs2, xfs,
hfsplus and nilfs2 - so I'd suggest that this is the best paaaaaaah
to take for implementing a generic mount option...

> At umount time, do we undo whatever overrides we set up at mount time?  Seems
> sane to me, just wanted to run it by everyone.

Does it really matter? The next mount will set it to whatever is
necessary...

> 3. Change the per-fs option handling code to call the same code as the VFS'
> nocacheflush option.  Any fs that wants to deprecate its per-fs option handler
> can do so.  Or they can stay forever.
> 
> 4. Remove all the flush conditionals from the fs code in favor of letting the
> block layer handle it.
>
> Hopefully "nocacheflush" is a little more obvious.

What cache does "nocacheflush" refer to? The page, inode, dentry, or
buffer caches? Or some other per filesystem cache? Perhaps the MD
stripe cache? Maybe something else? There are many different caches
in a storage system even before we consider hardware, so I think
"nocacheflush" is much more ambiguous than barrier/nobarrier...

Just my 2c worth....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/3] block: Create sysfs knobs to override FLUSH/FUA support flags

[Greg KH]

On Tue, Jan 25, 2011 at 11:16:26PM -0800, Darrick J. Wong wrote:
> This patch is the first in a series to refactor the barrier= mount options out
> of the filesystem code.  This patch adds sysfs knobs to disable flush and FUA;

If you add sysfs files, you must also add documentation for these files
in Documentation/ABI/.

Please do that in this patch series.

thanks,

greg k-h