diff mbox

Btrfs: use the new helper wbc_to_write_flags

Message ID 20170825001948.7759-1-bo.li.liu@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liu Bo Aug. 25, 2017, 12:19 a.m. UTC
This updates btrfs to use the helper wbc_to_write_flags which has been
applied in ext4/xfs/f2fs/block.

Please note that, with this, btrfs's dirty pages written by a
writeback job will carry the flag REQ_BACKGROUND, which is currently
used by writeback-throttle to determine whether it should go to get a
request or wait.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/extent_io.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

David Sterba Sept. 6, 2017, 2:38 p.m. UTC | #1
On Thu, Aug 24, 2017 at 06:19:48PM -0600, Liu Bo wrote:
> This updates btrfs to use the helper wbc_to_write_flags which has been
> applied in ext4/xfs/f2fs/block.

Added in commit 7637241e651ec36e4094 in 11/2016, I wonder why btrfs
wasn't been changed as well as it uses the same code patterns as the
other filesystems.

> Please note that, with this, btrfs's dirty pages written by a
> writeback job will carry the flag REQ_BACKGROUND, which is currently
> used by writeback-throttle to determine whether it should go to get a
> request or wait.

Which in my understanding of the WBT behaviour is what we want, right?

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

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

I'm still expecting some surprises (== performance drop) from the recent
WBT changes, as we still may miss some request flags, but more subtle
than eg. the superblock REQ_ tags.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Bo Sept. 6, 2017, 10:28 p.m. UTC | #2
On Wed, Sep 06, 2017 at 04:38:06PM +0200, David Sterba wrote:
> On Thu, Aug 24, 2017 at 06:19:48PM -0600, Liu Bo wrote:
> > This updates btrfs to use the helper wbc_to_write_flags which has been
> > applied in ext4/xfs/f2fs/block.
> 
> Added in commit 7637241e651ec36e4094 in 11/2016, I wonder why btrfs
> wasn't been changed as well as it uses the same code patterns as the
> other filesystems.
>

/me is curious, too.

> > Please note that, with this, btrfs's dirty pages written by a
> > writeback job will carry the flag REQ_BACKGROUND, which is currently
> > used by writeback-throttle to determine whether it should go to get a
> > request or wait.
> 
> Which in my understanding of the WBT behaviour is what we want, right?
>

Yes, it gives a hint to WBT whether it should wait or not.

> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> 
> Reviewed-by: David Sterba <dsterba@suse.com>
> 
> I'm still expecting some surprises (== performance drop) from the recent
> WBT changes, as we still may miss some request flags, but more subtle
> than eg. the superblock REQ_ tags.

Looks like this is the only missing flag I could find, the missing
REQ_META (in another patch) doesn't have an impact on WBT.

thanks,
-liubo
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a454a10..825fad6 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3467,8 +3467,7 @@  static int __extent_writepage(struct page *page, struct writeback_control *wbc,
 	int write_flags = 0;
 	unsigned long nr_written = 0;
 
-	if (wbc->sync_mode == WB_SYNC_ALL)
-		write_flags = REQ_SYNC;
+	write_flags = wbc_to_write_flags(wbc);
 
 	trace___extent_writepage(page, inode, wbc);
 
@@ -3713,7 +3712,7 @@  static noinline_for_stack int write_one_eb(struct extent_buffer *eb,
 	u32 nritems;
 	unsigned long i, num_pages;
 	unsigned long start, end;
-	int write_flags = (epd->sync_io ? REQ_SYNC : 0) | REQ_META;
+	int write_flags = wbc_to_write_flags(wbc) | REQ_META;
 	int ret = 0;
 
 	clear_bit(EXTENT_BUFFER_WRITE_ERR, &eb->bflags);