[PATCH 0/7] Fix fallout from changes to FUA and PREFLUSH definitions

[PATCH 0/7] Fix fallout from changes to FUA and PREFLUSH definitions

[Jan Kara]

Hello,

this series addresses a performance issue caused by commit b685d3d65ac7 "block:
treat REQ_FUA and REQ_PREFLUSH as synchronous". We know for certain this
problem significanly regresses (over 10%, in some cases up to 100%) ext4 and
btrfs for dbench4 and reaim benchmarks.  Based on this I have fixed up also
other places which suffer from the same problem however those changes are
untested so maintainers please have a look whether the change makes sense to
you and also whether I possibly didn't miss some cases where REQ_SYNC should be
also added. Patches in this series are completely independent so if maintainers
agree with the change, feel free to take it through your tree.

The core of the problem is that above mentioned commit removed REQ_SYNC flag
from WRITE_{FUA|PREFLUSH|...} definitions.  generic_make_request_checks()
however strips REQ_FUA and REQ_PREFLUSH flags from a bio when the storage
doesn't report volatile write cache and thus write effectively becomes
asynchronous which can lead to performance regressions.

A side note for ext4: The two patches for ext4 & jbd2 are on top of the change
that got merged in the ext4 tree already.

								Honza

[PATCH 1/7] ext4: Mark superblock writes synchronous for nobarrier mounts

[Jan Kara]

Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
synchronous" removed REQ_SYNC flag from WRITE_FUA implementation.
generic_make_request_checks() however strips REQ_FUA flag from a bio
when the storage doesn't report volatile write cache and thus write
effectively becomes asynchronous which can lead to performance
regressions. This affects superblock writes for ext4. Fix the problem
by marking superblock writes always as synchronous.

Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
CC: linux-ext4@vger.kernel.org
CC: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a9448db1cf7e..f902852ac13e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4629,7 +4629,7 @@ static int ext4_commit_super(struct super_block *sb, int sync)
 	if (sync) {
 		unlock_buffer(sbh);
 		error = __sync_dirty_buffer(sbh,
-			test_opt(sb, BARRIER) ? REQ_FUA : REQ_SYNC);
+			REQ_SYNC | (test_opt(sb, BARRIER) ? REQ_FUA : 0));
 		if (error)
 			return error;
 
-- 
2.12.0

Re: [PATCH 1/7] ext4: Mark superblock writes synchronous for nobarrier mounts

[Theodore Ts'o]

On Tue, May 02, 2017 at 05:03:45PM +0200, Jan Kara wrote:
> Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> synchronous" removed REQ_SYNC flag from WRITE_FUA implementation.
> generic_make_request_checks() however strips REQ_FUA flag from a bio
> when the storage doesn't report volatile write cache and thus write
> effectively becomes asynchronous which can lead to performance
> regressions. This affects superblock writes for ext4. Fix the problem
> by marking superblock writes always as synchronous.
> 
> Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
> CC: linux-ext4@vger.kernel.org
> CC: "Theodore Ts'o" <tytso@mit.edu>
> Signed-off-by: Jan Kara <jack@suse.cz>

Thanks, applied.

						- Ted

[PATCH 2/7] jbd2: Cleanup write flags handling from jbd2_write_superblock()

[Jan Kara]

Currently jbd2_write_superblock() silently adds REQ_SYNC to flags with
which journal superblock is written. Make this explicit by making flags
passed down to jbd2_write_superblock() contain REQ_SYNC.

CC: "Theodore Ts'o" <tytso@mit.edu>
CC: linux-ext4@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/jbd2/journal.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index e768126f6a72..cc2038f2f41a 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -913,7 +913,8 @@ int __jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block)
 	 * space and if we lose sb update during power failure we'd replay
 	 * old transaction with possibly newly overwritten data.
 	 */
-	ret = jbd2_journal_update_sb_log_tail(journal, tid, block, REQ_FUA);
+	ret = jbd2_journal_update_sb_log_tail(journal, tid, block,
+					      REQ_SYNC | REQ_FUA);
 	if (ret)
 		goto out;
 
@@ -1314,7 +1315,7 @@ static int journal_reset(journal_t *journal)
 		jbd2_journal_update_sb_log_tail(journal,
 						journal->j_tail_sequence,
 						journal->j_tail,
-						REQ_FUA);
+						REQ_SYNC | REQ_FUA);
 		mutex_unlock(&journal->j_checkpoint_mutex);
 	}
 	return jbd2_journal_start_thread(journal);
@@ -1348,7 +1349,7 @@ static int jbd2_write_superblock(journal_t *journal, int write_flags)
 	jbd2_superblock_csum_set(journal, sb);
 	get_bh(bh);
 	bh->b_end_io = end_buffer_write_sync;
-	ret = submit_bh(REQ_OP_WRITE, write_flags | REQ_SYNC, bh);
+	ret = submit_bh(REQ_OP_WRITE, write_flags, bh);
 	wait_on_buffer(bh);
 	if (buffer_write_io_error(bh)) {
 		clear_buffer_write_io_error(bh);
@@ -1454,7 +1455,7 @@ void jbd2_journal_update_sb_errno(journal_t *journal)
 	sb->s_errno    = cpu_to_be32(journal->j_errno);
 	read_unlock(&journal->j_state_lock);
 
-	jbd2_write_superblock(journal, REQ_FUA);
+	jbd2_write_superblock(journal, REQ_SYNC | REQ_FUA);
 }
 EXPORT_SYMBOL(jbd2_journal_update_sb_errno);
 
@@ -1721,7 +1722,7 @@ int jbd2_journal_destroy(journal_t *journal)
 			write_unlock(&journal->j_state_lock);
 
 			jbd2_mark_journal_empty(journal,
-					REQ_PREFLUSH | REQ_FUA);
+					REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
 			mutex_unlock(&journal->j_checkpoint_mutex);
 		} else
 			err = -EIO;
@@ -1980,7 +1981,7 @@ int jbd2_journal_flush(journal_t *journal)
 	 * the magic code for a fully-recovered superblock.  Any future
 	 * commits of data to the journal will restore the current
 	 * s_start value. */
-	jbd2_mark_journal_empty(journal, REQ_FUA);
+	jbd2_mark_journal_empty(journal, REQ_SYNC | REQ_FUA);
 	mutex_unlock(&journal->j_checkpoint_mutex);
 	write_lock(&journal->j_state_lock);
 	J_ASSERT(!journal->j_running_transaction);
@@ -2026,7 +2027,7 @@ int jbd2_journal_wipe(journal_t *journal, int write)
 	if (write) {
 		/* Lock to make assertions happy... */
 		mutex_lock(&journal->j_checkpoint_mutex);
-		jbd2_mark_journal_empty(journal, REQ_FUA);
+		jbd2_mark_journal_empty(journal, REQ_SYNC | REQ_FUA);
 		mutex_unlock(&journal->j_checkpoint_mutex);
 	}
 
-- 
2.12.0

Re: [PATCH 2/7] jbd2: Cleanup write flags handling from jbd2_write_superblock()

[Theodore Ts'o]

On Tue, May 02, 2017 at 05:03:46PM +0200, Jan Kara wrote:
> Currently jbd2_write_superblock() silently adds REQ_SYNC to flags with
> which journal superblock is written. Make this explicit by making flags
> passed down to jbd2_write_superblock() contain REQ_SYNC.
> 
> CC: "Theodore Ts'o" <tytso@mit.edu>
> CC: linux-ext4@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>

Thanks, applied.

						- Ted

[PATCH 3/7] f2fs: Make flush bios explicitely sync

[Jan Kara]

Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
definitions.  generic_make_request_checks() however strips REQ_FUA and
REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
write cache and thus write effectively becomes asynchronous which can
lead to performance regressions.

Fix the problem by making sure all bios which are synchronous are
properly marked with REQ_SYNC.

Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
CC: Jaegeuk Kim <jaegeuk@kernel.org>
CC: linux-f2fs-devel@lists.sourceforge.net
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/f2fs/data.c    | 2 +-
 fs/f2fs/segment.c | 2 +-
 fs/f2fs/super.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 1602b4bccae6..dd24476e8d2c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -309,7 +309,7 @@ static void __f2fs_submit_merged_bio(struct f2fs_sb_info *sbi,
 	if (type >= META_FLUSH) {
 		io->fio.type = META_FLUSH;
 		io->fio.op = REQ_OP_WRITE;
-		io->fio.op_flags = REQ_META | REQ_PRIO;
+		io->fio.op_flags = REQ_META | REQ_PRIO | REQ_SYNC;
 		if (!test_opt(sbi, NOBARRIER))
 			io->fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
 	}
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 29ef7088c558..0c24cec55dc9 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -416,7 +416,7 @@ static int __submit_flush_wait(struct block_device *bdev)
 	struct bio *bio = f2fs_bio_alloc(0);
 	int ret;
 
-	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
+	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH;
 	bio->bi_bdev = bdev;
 	ret = submit_bio_wait(bio);
 	bio_put(bio);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 96fe8ed73100..fa5af1f04798 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1307,7 +1307,7 @@ static int __f2fs_commit_super(struct buffer_head *bh,
 	unlock_buffer(bh);
 
 	/* it's rare case, we can do fua all the time */
-	return __sync_dirty_buffer(bh, REQ_PREFLUSH | REQ_FUA);
+	return __sync_dirty_buffer(bh, REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
 }
 
 static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
-- 
2.12.0

Re: [f2fs-dev] [PATCH 3/7] f2fs: Make flush bios explicitely sync

[Chao Yu]

On 2017/5/2 23:03, Jan Kara wrote:
> Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
> definitions.  generic_make_request_checks() however strips REQ_FUA and
> REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
> write cache and thus write effectively becomes asynchronous which can
> lead to performance regressions.
> 
> Fix the problem by making sure all bios which are synchronous are
> properly marked with REQ_SYNC.
> 
> Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
> CC: Jaegeuk Kim <jaegeuk@kernel.org>
> CC: linux-f2fs-devel@lists.sourceforge.net
> Signed-off-by: Jan Kara <jack@suse.cz>

Looks good to me. :)

Acked-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> ---
>  fs/f2fs/data.c    | 2 +-
>  fs/f2fs/segment.c | 2 +-
>  fs/f2fs/super.c   | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 1602b4bccae6..dd24476e8d2c 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -309,7 +309,7 @@ static void __f2fs_submit_merged_bio(struct f2fs_sb_info *sbi,
>  	if (type >= META_FLUSH) {
>  		io->fio.type = META_FLUSH;
>  		io->fio.op = REQ_OP_WRITE;
> -		io->fio.op_flags = REQ_META | REQ_PRIO;
> +		io->fio.op_flags = REQ_META | REQ_PRIO | REQ_SYNC;
>  		if (!test_opt(sbi, NOBARRIER))
>  			io->fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
>  	}
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 29ef7088c558..0c24cec55dc9 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -416,7 +416,7 @@ static int __submit_flush_wait(struct block_device *bdev)
>  	struct bio *bio = f2fs_bio_alloc(0);
>  	int ret;
>  
> -	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
> +	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH;
>  	bio->bi_bdev = bdev;
>  	ret = submit_bio_wait(bio);
>  	bio_put(bio);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 96fe8ed73100..fa5af1f04798 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1307,7 +1307,7 @@ static int __f2fs_commit_super(struct buffer_head *bh,
>  	unlock_buffer(bh);
>  
>  	/* it's rare case, we can do fua all the time */
> -	return __sync_dirty_buffer(bh, REQ_PREFLUSH | REQ_FUA);
> +	return __sync_dirty_buffer(bh, REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
>  }
>  
>  static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
> 

Re: [PATCH 3/7] f2fs: Make flush bios explicitely sync

[Jaegeuk Kim]

Hi Jan,

On 05/02, Jan Kara wrote:
> Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
> definitions.  generic_make_request_checks() however strips REQ_FUA and
> REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
> write cache and thus write effectively becomes asynchronous which can
> lead to performance regressions.
> 
> Fix the problem by making sure all bios which are synchronous are
> properly marked with REQ_SYNC.
> 
> Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3

Looks good to me. Thank you for the patch.
BTW, does this need cc to stable?

Cc: stable@vger.kernel.org # 4.9+

Thanks,

> CC: Jaegeuk Kim <jaegeuk@kernel.org>
> CC: linux-f2fs-devel@lists.sourceforge.net
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/f2fs/data.c    | 2 +-
>  fs/f2fs/segment.c | 2 +-
>  fs/f2fs/super.c   | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 1602b4bccae6..dd24476e8d2c 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -309,7 +309,7 @@ static void __f2fs_submit_merged_bio(struct f2fs_sb_info *sbi,
>  	if (type >= META_FLUSH) {
>  		io->fio.type = META_FLUSH;
>  		io->fio.op = REQ_OP_WRITE;
> -		io->fio.op_flags = REQ_META | REQ_PRIO;
> +		io->fio.op_flags = REQ_META | REQ_PRIO | REQ_SYNC;
>  		if (!test_opt(sbi, NOBARRIER))
>  			io->fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
>  	}
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 29ef7088c558..0c24cec55dc9 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -416,7 +416,7 @@ static int __submit_flush_wait(struct block_device *bdev)
>  	struct bio *bio = f2fs_bio_alloc(0);
>  	int ret;
>  
> -	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
> +	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH;
>  	bio->bi_bdev = bdev;
>  	ret = submit_bio_wait(bio);
>  	bio_put(bio);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 96fe8ed73100..fa5af1f04798 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1307,7 +1307,7 @@ static int __f2fs_commit_super(struct buffer_head *bh,
>  	unlock_buffer(bh);
>  
>  	/* it's rare case, we can do fua all the time */
> -	return __sync_dirty_buffer(bh, REQ_PREFLUSH | REQ_FUA);
> +	return __sync_dirty_buffer(bh, REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
>  }
>  
>  static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
> -- 
> 2.12.0

Re: [PATCH 3/7] f2fs: Make flush bios explicitely sync

[Jan Kara]

On Wed 03-05-17 14:22:46, Jaegeuk Kim wrote:
> Hi Jan,
> 
> On 05/02, Jan Kara wrote:
> > Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> > synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
> > definitions.  generic_make_request_checks() however strips REQ_FUA and
> > REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
> > write cache and thus write effectively becomes asynchronous which can
> > lead to performance regressions.
> > 
> > Fix the problem by making sure all bios which are synchronous are
> > properly marked with REQ_SYNC.
> > 
> > Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
> 
> Looks good to me. Thank you for the patch.
> BTW, does this need cc to stable?
> 
> Cc: stable@vger.kernel.org # 4.9+

Yeah, good point. Added.

								Honza
> 
> Thanks,
> 
> > CC: Jaegeuk Kim <jaegeuk@kernel.org>
> > CC: linux-f2fs-devel@lists.sourceforge.net
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/f2fs/data.c    | 2 +-
> >  fs/f2fs/segment.c | 2 +-
> >  fs/f2fs/super.c   | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 1602b4bccae6..dd24476e8d2c 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -309,7 +309,7 @@ static void __f2fs_submit_merged_bio(struct f2fs_sb_info *sbi,
> >  	if (type >= META_FLUSH) {
> >  		io->fio.type = META_FLUSH;
> >  		io->fio.op = REQ_OP_WRITE;
> > -		io->fio.op_flags = REQ_META | REQ_PRIO;
> > +		io->fio.op_flags = REQ_META | REQ_PRIO | REQ_SYNC;
> >  		if (!test_opt(sbi, NOBARRIER))
> >  			io->fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
> >  	}
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 29ef7088c558..0c24cec55dc9 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -416,7 +416,7 @@ static int __submit_flush_wait(struct block_device *bdev)
> >  	struct bio *bio = f2fs_bio_alloc(0);
> >  	int ret;
> >  
> > -	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
> > +	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH;
> >  	bio->bi_bdev = bdev;
> >  	ret = submit_bio_wait(bio);
> >  	bio_put(bio);
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 96fe8ed73100..fa5af1f04798 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -1307,7 +1307,7 @@ static int __f2fs_commit_super(struct buffer_head *bh,
> >  	unlock_buffer(bh);
> >  
> >  	/* it's rare case, we can do fua all the time */
> > -	return __sync_dirty_buffer(bh, REQ_PREFLUSH | REQ_FUA);
> > +	return __sync_dirty_buffer(bh, REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
> >  }
> >  
> >  static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
> > -- 
> > 2.12.0
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/7] f2fs: Make flush bios explicitely sync

[Jan Kara]

On Thu 04-05-17 09:32:11, Jan Kara wrote:
> On Wed 03-05-17 14:22:46, Jaegeuk Kim wrote:
> > Hi Jan,
> > 
> > On 05/02, Jan Kara wrote:
> > > Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> > > synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
> > > definitions.  generic_make_request_checks() however strips REQ_FUA and
> > > REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
> > > write cache and thus write effectively becomes asynchronous which can
> > > lead to performance regressions.
> > > 
> > > Fix the problem by making sure all bios which are synchronous are
> > > properly marked with REQ_SYNC.
> > > 
> > > Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
> > 
> > Looks good to me. Thank you for the patch.
> > BTW, does this need cc to stable?
> > 
> > Cc: stable@vger.kernel.org # 4.9+
> 
> Yeah, good point. Added.

BTW: Will you take the patch through your tree or should I take care of
merging it?

								Honza

> > > CC: Jaegeuk Kim <jaegeuk@kernel.org>
> > > CC: linux-f2fs-devel@lists.sourceforge.net
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > > ---
> > >  fs/f2fs/data.c    | 2 +-
> > >  fs/f2fs/segment.c | 2 +-
> > >  fs/f2fs/super.c   | 2 +-
> > >  3 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index 1602b4bccae6..dd24476e8d2c 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -309,7 +309,7 @@ static void __f2fs_submit_merged_bio(struct f2fs_sb_info *sbi,
> > >  	if (type >= META_FLUSH) {
> > >  		io->fio.type = META_FLUSH;
> > >  		io->fio.op = REQ_OP_WRITE;
> > > -		io->fio.op_flags = REQ_META | REQ_PRIO;
> > > +		io->fio.op_flags = REQ_META | REQ_PRIO | REQ_SYNC;
> > >  		if (!test_opt(sbi, NOBARRIER))
> > >  			io->fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
> > >  	}
> > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > index 29ef7088c558..0c24cec55dc9 100644
> > > --- a/fs/f2fs/segment.c
> > > +++ b/fs/f2fs/segment.c
> > > @@ -416,7 +416,7 @@ static int __submit_flush_wait(struct block_device *bdev)
> > >  	struct bio *bio = f2fs_bio_alloc(0);
> > >  	int ret;
> > >  
> > > -	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
> > > +	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH;
> > >  	bio->bi_bdev = bdev;
> > >  	ret = submit_bio_wait(bio);
> > >  	bio_put(bio);
> > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > > index 96fe8ed73100..fa5af1f04798 100644
> > > --- a/fs/f2fs/super.c
> > > +++ b/fs/f2fs/super.c
> > > @@ -1307,7 +1307,7 @@ static int __f2fs_commit_super(struct buffer_head *bh,
> > >  	unlock_buffer(bh);
> > >  
> > >  	/* it's rare case, we can do fua all the time */
> > > -	return __sync_dirty_buffer(bh, REQ_PREFLUSH | REQ_FUA);
> > > +	return __sync_dirty_buffer(bh, REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
> > >  }
> > >  
> > >  static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
> > > -- 
> > > 2.12.0
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/7] f2fs: Make flush bios explicitely sync

[Jaegeuk Kim]

On 05/04, Jan Kara wrote:
> On Thu 04-05-17 09:32:11, Jan Kara wrote:
> > On Wed 03-05-17 14:22:46, Jaegeuk Kim wrote:
> > > Hi Jan,
> > > 
> > > On 05/02, Jan Kara wrote:
> > > > Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> > > > synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
> > > > definitions.  generic_make_request_checks() however strips REQ_FUA and
> > > > REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
> > > > write cache and thus write effectively becomes asynchronous which can
> > > > lead to performance regressions.
> > > > 
> > > > Fix the problem by making sure all bios which are synchronous are
> > > > properly marked with REQ_SYNC.
> > > > 
> > > > Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
> > > 
> > > Looks good to me. Thank you for the patch.
> > > BTW, does this need cc to stable?
> > > 
> > > Cc: stable@vger.kernel.org # 4.9+
> > 
> > Yeah, good point. Added.
> 
> BTW: Will you take the patch through your tree or should I take care of
> merging it?

Let me merge this patch with cc, if you don't mind.

Thanks,

> 
> 								Honza
> 
> > > > CC: Jaegeuk Kim <jaegeuk@kernel.org>
> > > > CC: linux-f2fs-devel@lists.sourceforge.net
> > > > Signed-off-by: Jan Kara <jack@suse.cz>
> > > > ---
> > > >  fs/f2fs/data.c    | 2 +-
> > > >  fs/f2fs/segment.c | 2 +-
> > > >  fs/f2fs/super.c   | 2 +-
> > > >  3 files changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > > index 1602b4bccae6..dd24476e8d2c 100644
> > > > --- a/fs/f2fs/data.c
> > > > +++ b/fs/f2fs/data.c
> > > > @@ -309,7 +309,7 @@ static void __f2fs_submit_merged_bio(struct f2fs_sb_info *sbi,
> > > >  	if (type >= META_FLUSH) {
> > > >  		io->fio.type = META_FLUSH;
> > > >  		io->fio.op = REQ_OP_WRITE;
> > > > -		io->fio.op_flags = REQ_META | REQ_PRIO;
> > > > +		io->fio.op_flags = REQ_META | REQ_PRIO | REQ_SYNC;
> > > >  		if (!test_opt(sbi, NOBARRIER))
> > > >  			io->fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
> > > >  	}
> > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > > index 29ef7088c558..0c24cec55dc9 100644
> > > > --- a/fs/f2fs/segment.c
> > > > +++ b/fs/f2fs/segment.c
> > > > @@ -416,7 +416,7 @@ static int __submit_flush_wait(struct block_device *bdev)
> > > >  	struct bio *bio = f2fs_bio_alloc(0);
> > > >  	int ret;
> > > >  
> > > > -	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
> > > > +	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH;
> > > >  	bio->bi_bdev = bdev;
> > > >  	ret = submit_bio_wait(bio);
> > > >  	bio_put(bio);
> > > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > > > index 96fe8ed73100..fa5af1f04798 100644
> > > > --- a/fs/f2fs/super.c
> > > > +++ b/fs/f2fs/super.c
> > > > @@ -1307,7 +1307,7 @@ static int __f2fs_commit_super(struct buffer_head *bh,
> > > >  	unlock_buffer(bh);
> > > >  
> > > >  	/* it's rare case, we can do fua all the time */
> > > > -	return __sync_dirty_buffer(bh, REQ_PREFLUSH | REQ_FUA);
> > > > +	return __sync_dirty_buffer(bh, REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
> > > >  }
> > > >  
> > > >  static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
> > > > -- 
> > > > 2.12.0
> > -- 
> > Jan Kara <jack@suse.com>
> > SUSE Labs, CR
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

[PATCH 4/7] gfs2: Make flush bios explicitely sync

[Jan Kara]

Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
definitions.  generic_make_request_checks() however strips REQ_FUA and
REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
write cache and thus write effectively becomes asynchronous which can
lead to performance regressions

Fix the problem by making sure all bios which are synchronous are
properly marked with REQ_SYNC.

Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
CC: Steven Whitehouse <swhiteho@redhat.com>
CC: cluster-devel@redhat.com
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/gfs2/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index f865b96374df..d2955daf17a4 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -659,7 +659,7 @@ static void log_write_header(struct gfs2_sbd *sdp, u32 flags)
 	struct gfs2_log_header *lh;
 	unsigned int tail;
 	u32 hash;
-	int op_flags = REQ_PREFLUSH | REQ_FUA | REQ_META;
+	int op_flags = REQ_PREFLUSH | REQ_FUA | REQ_META | REQ_SYNC;
 	struct page *page = mempool_alloc(gfs2_page_pool, GFP_NOIO);
 	enum gfs2_freeze_state state = atomic_read(&sdp->sd_freeze_state);
 	lh = page_address(page);
-- 
2.12.0

Re: [PATCH 4/7] gfs2: Make flush bios explicitely sync

[Bob Peterson]

----- Original Message -----
| Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
| synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
| definitions.  generic_make_request_checks() however strips REQ_FUA and
| REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
| write cache and thus write effectively becomes asynchronous which can
| lead to performance regressions
| 
| Fix the problem by making sure all bios which are synchronous are
| properly marked with REQ_SYNC.

Hi,

Looks good.
Acked-by: Bob Peterson <rpeterso@redhat.com>

Regards,

Bob Peterson
Red Hat File Systems

[PATCH 5/7] reiserfs: Make flush bios explicitely sync

[Jan Kara]

Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
definitions.  generic_make_request_checks() however strips REQ_FUA and
REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
write cache and thus write effectively becomes asynchronous which can
lead to performance regressions

Fix the problem by making sure all bios which are synchronous are
properly marked with REQ_SYNC.

Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
CC: reiserfs-devel@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/reiserfs/journal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
index aa40c242f1db..64a4d3c82125 100644
--- a/fs/reiserfs/journal.c
+++ b/fs/reiserfs/journal.c
@@ -1112,7 +1112,7 @@ static int flush_commit_list(struct super_block *s,
 		depth = reiserfs_write_unlock_nested(s);
 		if (reiserfs_barrier_flush(s))
 			__sync_dirty_buffer(jl->j_commit_bh,
-					REQ_PREFLUSH | REQ_FUA);
+					REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
 		else
 			sync_dirty_buffer(jl->j_commit_bh);
 		reiserfs_write_lock_nested(s, depth);
@@ -1271,7 +1271,7 @@ static int _update_journal_header_block(struct super_block *sb,
 
 		if (reiserfs_barrier_flush(sb))
 			__sync_dirty_buffer(journal->j_header_bh,
-					REQ_PREFLUSH | REQ_FUA);
+					REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
 		else
 			sync_dirty_buffer(journal->j_header_bh);
 
-- 
2.12.0

[PATCH 6/7] btrfs: Make flush bios explicitely sync

[Jan Kara]

Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
definitions.  generic_make_request_checks() however strips REQ_FUA and
REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
write cache and thus write effectively becomes asynchronous which can
lead to performance regressions

Fix the problem by making sure all bios which are synchronous are
properly marked with REQ_SYNC.

CC: David Sterba <dsterba@suse.com>
CC: linux-btrfs@vger.kernel.org
Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/btrfs/disk-io.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index eb1ee7b6f532..af75a9aab81e 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3488,10 +3488,12 @@ static int write_dev_supers(struct btrfs_device *device,
 		 * we fua the first super.  The others we allow
 		 * to go down lazy.
 		 */
-		if (i == 0)
-			ret = btrfsic_submit_bh(REQ_OP_WRITE, REQ_FUA, bh);
-		else
+		if (i == 0) {
+			ret = btrfsic_submit_bh(REQ_OP_WRITE,
+						REQ_SYNC | REQ_FUA, bh);
+		} else {
 			ret = btrfsic_submit_bh(REQ_OP_WRITE, REQ_SYNC, bh);
+		}
 		if (ret)
 			errors++;
 	}
@@ -3555,7 +3557,7 @@ static int write_dev_flush(struct btrfs_device *device, int wait)
 
 	bio->bi_end_io = btrfs_end_empty_barrier;
 	bio->bi_bdev = device->bdev;
-	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
+	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH;
 	init_completion(&device->flush_wait);
 	bio->bi_private = &device->flush_wait;
 	device->flush_bio = bio;
-- 
2.12.0

Re: [PATCH 6/7] btrfs: Make flush bios explicitely sync

[David Sterba]

On Tue, May 02, 2017 at 05:03:50PM +0200, Jan Kara wrote:
> Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
> definitions.  generic_make_request_checks() however strips REQ_FUA and
> REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
> write cache and thus write effectively becomes asynchronous which can
> lead to performance regressions
> 
> Fix the problem by making sure all bios which are synchronous are
> properly marked with REQ_SYNC.
> 
> CC: David Sterba <dsterba@suse.com>
> CC: linux-btrfs@vger.kernel.org
> Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: David Sterba <dsterba@suse.com>

Re: [PATCH 6/7] btrfs: Make flush bios explicitely sync

[Liu Bo]

On Tue, May 02, 2017 at 05:03:50PM +0200, Jan Kara wrote:
> Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
> definitions.  generic_make_request_checks() however strips REQ_FUA and
> REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
> write cache and thus write effectively becomes asynchronous which can
> lead to performance regressions
> 
> Fix the problem by making sure all bios which are synchronous are
> properly marked with REQ_SYNC.
>

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

Thanks,

-liubo
> CC: David Sterba <dsterba@suse.com>
> CC: linux-btrfs@vger.kernel.org
> Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/btrfs/disk-io.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index eb1ee7b6f532..af75a9aab81e 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3488,10 +3488,12 @@ static int write_dev_supers(struct btrfs_device *device,
>  		 * we fua the first super.  The others we allow
>  		 * to go down lazy.
>  		 */
> -		if (i == 0)
> -			ret = btrfsic_submit_bh(REQ_OP_WRITE, REQ_FUA, bh);
> -		else
> +		if (i == 0) {
> +			ret = btrfsic_submit_bh(REQ_OP_WRITE,
> +						REQ_SYNC | REQ_FUA, bh);
> +		} else {
>  			ret = btrfsic_submit_bh(REQ_OP_WRITE, REQ_SYNC, bh);
> +		}
>  		if (ret)
>  			errors++;
>  	}
> @@ -3555,7 +3557,7 @@ static int write_dev_flush(struct btrfs_device *device, int wait)
>  
>  	bio->bi_end_io = btrfs_end_empty_barrier;
>  	bio->bi_bdev = device->bdev;
> -	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
> +	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH;
>  	init_completion(&device->flush_wait);
>  	bio->bi_private = &device->flush_wait;
>  	device->flush_bio = bio;
> -- 
> 2.12.0
> 

[PATCH 7/7] md: Make flush bios explicitely sync

[Jan Kara]

Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
definitions.  generic_make_request_checks() however strips REQ_FUA and
REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
write cache and thus write effectively becomes asynchronous which can
lead to performance regressions

Fix the problem by making sure all bios which are synchronous are
properly marked with REQ_SYNC.

CC: linux-raid@vger.kernel.org
CC: Shaohua Li <shli@kernel.org>
CC: Mike Snitzer <snitzer@redhat.com>
CC: dm-devel@redhat.com
Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
Signed-off-by: Jan Kara <jack@suse.cz>
---
 drivers/md/dm-snap-persistent.c | 3 ++-
 drivers/md/md.c                 | 2 +-
 drivers/md/raid5-cache.c        | 4 ++--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c
index b93476c3ba3f..b92ab4cb0710 100644
--- a/drivers/md/dm-snap-persistent.c
+++ b/drivers/md/dm-snap-persistent.c
@@ -741,7 +741,8 @@ static void persistent_commit_exception(struct dm_exception_store *store,
 	/*
 	 * Commit exceptions to disk.
 	 */
-	if (ps->valid && area_io(ps, REQ_OP_WRITE, REQ_PREFLUSH | REQ_FUA))
+	if (ps->valid && area_io(ps, REQ_OP_WRITE,
+				 REQ_SYNC | REQ_PREFLUSH | REQ_FUA))
 		ps->valid = 0;
 
 	/*
diff --git a/drivers/md/md.c b/drivers/md/md.c
index f6ae1d67bcd0..9c40ce0642c2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -753,7 +753,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev,
 	    test_bit(FailFast, &rdev->flags) &&
 	    !test_bit(LastDev, &rdev->flags))
 		ff = MD_FAILFAST;
-	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH | REQ_FUA | ff;
+	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH | REQ_FUA | ff;
 
 	atomic_inc(&mddev->pending_writes);
 	submit_bio(bio);
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 3f307be01b10..0bd5d0c88cee 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -1625,7 +1625,7 @@ static int r5l_log_write_empty_meta_block(struct r5l_log *log, sector_t pos,
 	mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum,
 					     mb, PAGE_SIZE));
 	if (!sync_page_io(log->rdev, pos, PAGE_SIZE, page, REQ_OP_WRITE,
-			  REQ_FUA, false)) {
+			  REQ_SYNC | REQ_FUA, false)) {
 		__free_page(page);
 		return -EIO;
 	}
@@ -2198,7 +2198,7 @@ r5c_recovery_rewrite_data_only_stripes(struct r5l_log *log,
 		mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum,
 						     mb, PAGE_SIZE));
 		sync_page_io(log->rdev, ctx->pos, PAGE_SIZE, page,
-			     REQ_OP_WRITE, REQ_FUA, false);
+			     REQ_OP_WRITE, REQ_SYNC | REQ_FUA, false);
 		sh->log_start = ctx->pos;
 		list_add_tail(&sh->r5c, &log->stripe_in_journal_list);
 		atomic_inc(&log->stripe_in_journal_count);
-- 
2.12.0