Linux-Fsdevel Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Fixes and cleanups to fs-writeback
@ 2024-02-28  9:19 Kemeng Shi
  2024-02-28  9:19 ` [PATCH v2 1/6] fs/writeback: avoid to writeback non-expired inode in kupdate writeback Kemeng Shi
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Kemeng Shi @ 2024-02-28  9:19 UTC (permalink / raw
  To: viro, brauner, jack, tim.c.chen; +Cc: linux-fsdevel, linux-kernel

v1->v2:
-Filter non-expired in requeue_inode in patch "fs/writeback: avoid to
writeback non-expired inode in kupdate writeback"
-Wrap the comment at 80 columns in patch "fs/writeback: only calculate
dirtied_before when b_io is empty"
-Abandon patch "fs/writeback: remove unneeded check in
writeback_single_inode"
-Collect RVB from Jan and Tim

Kemeng Shi (6):
  fs/writeback: avoid to writeback non-expired inode in kupdate
    writeback
  fs/writeback: bail out if there is no more inodes for IO and queued
    once
  fs/writeback: remove unused parameter wb of finish_writeback_work
  fs/writeback: only calculate dirtied_before when b_io is empty
  fs/writeback: correct comment of __wakeup_flusher_threads_bdi
  fs/writeback: remove unnecessary return in writeback_inodes_sb

 fs/fs-writeback.c | 57 +++++++++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 24 deletions(-)

-- 
2.30.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/6] fs/writeback: avoid to writeback non-expired inode in kupdate writeback
  2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
@ 2024-02-28  9:19 ` Kemeng Shi
  2024-03-18 17:07   ` Jan Kara
  2024-02-28  9:19 ` [PATCH v2 2/6] fs/writeback: bail out if there is no more inodes for IO and queued once Kemeng Shi
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Kemeng Shi @ 2024-02-28  9:19 UTC (permalink / raw
  To: viro, brauner, jack, tim.c.chen; +Cc: linux-fsdevel, linux-kernel

In kupdate writeback, only expired inode (have been dirty for longer than
dirty_expire_interval) is supposed to be written back. However, kupdate
writeback will writeback non-expired inode left in b_io or b_more_io from
last wb_writeback. As a result, writeback will keep being triggered
unexpected when we keep dirtying pages even dirty memory is under
threshold and inode is not expired. To be more specific:
Assume dirty background threshold is > 1G and dirty_expire_centisecs is
> 60s. When we running fio -size=1G -invalidate=0 -ioengine=libaio
--time_based -runtime=60... (keep dirtying), the writeback will keep
being triggered as following:
wb_workfn
  wb_do_writeback
    wb_check_background_flush
      /*
       * Wb dirty background threshold starts at 0 if device was idle and
       * grows up when bandwidth of wb is updated. So a background
       * writeback is triggered.
       */
      wb_over_bg_thresh
      /*
       * Dirtied inode will be written back and added to b_more_io list
       * after slice used up (because we keep dirtying the inode).
       */
      wb_writeback

Writeback is triggered per dirty_writeback_centisecs as following:
wb_workfn
  wb_do_writeback
    wb_check_old_data_flush
      /*
       * Write back inode left in b_io and b_more_io from last wb_writeback
       * even the inode is non-expired and it will be added to b_more_io
       * again as slice will be used up (because we keep dirtying the
       * inode)
       */
      wb_writeback

Fix this by moving non-expired inode to dirty list instead of more io
list for kupdate writeback in requeue_inode.

Test as following:
/* make it more easier to observe the issue */
echo 300000 > /proc/sys/vm/dirty_expire_centisecs
echo 100 > /proc/sys/vm/dirty_writeback_centisecs
/* create a idle device */
mkfs.ext4 -F /dev/vdb
mount /dev/vdb /bdi1/
/* run buffer write with fio */
fio -name test -filename=/bdi1/file -size=800M -ioengine=libaio -bs=4K \
-iodepth=1 -rw=write -direct=0 --time_based -runtime=60 -invalidate=0

Fio result before fix (run three tests):
1360MB/s
1329MB/s
1455MB/s

Fio result after fix (run three tests):
1737MB/s
1729MB/s
1789MB/s

Writeback for non-expired inode is gone as expeted. Observe this with trace
writeback_start and writeback_written as following:
echo 1 > /sys/kernel/debug/tracing/events/writeback/writeback_start/enab
echo 1 > /sys/kernel/debug/tracing/events/writeback/writeback_written/enable
cat /sys/kernel/tracing/trace_pipe

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/fs-writeback.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 5ab1aaf805f7..4e6166e07eaf 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1561,7 +1561,8 @@ static void inode_sleep_on_writeback(struct inode *inode)
  * thread's back can have unexpected consequences.
  */
 static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
-			  struct writeback_control *wbc)
+			  struct writeback_control *wbc,
+			  unsigned long dirtied_before)
 {
 	if (inode->i_state & I_FREEING)
 		return;
@@ -1594,7 +1595,8 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
 		 * We didn't write back all the pages.  nfs_writepages()
 		 * sometimes bales out without doing anything.
 		 */
-		if (wbc->nr_to_write <= 0) {
+		if (wbc->nr_to_write <= 0 &&
+		    !inode_dirtied_after(inode, dirtied_before)) {
 			/* Slice used up. Queue for next turn. */
 			requeue_io(inode, wb);
 		} else {
@@ -1862,6 +1864,11 @@ static long writeback_sb_inodes(struct super_block *sb,
 	unsigned long start_time = jiffies;
 	long write_chunk;
 	long total_wrote = 0;  /* count both pages and inodes */
+	unsigned long dirtied_before = jiffies;
+
+	if (work->for_kupdate)
+		dirtied_before = jiffies -
+			msecs_to_jiffies(dirty_expire_interval * 10);
 
 	while (!list_empty(&wb->b_io)) {
 		struct inode *inode = wb_inode(wb->b_io.prev);
@@ -1967,7 +1974,7 @@ static long writeback_sb_inodes(struct super_block *sb,
 		spin_lock(&inode->i_lock);
 		if (!(inode->i_state & I_DIRTY_ALL))
 			total_wrote++;
-		requeue_inode(inode, tmp_wb, &wbc);
+		requeue_inode(inode, tmp_wb, &wbc, dirtied_before);
 		inode_sync_complete(inode);
 		spin_unlock(&inode->i_lock);
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/6] fs/writeback: bail out if there is no more inodes for IO and queued once
  2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
  2024-02-28  9:19 ` [PATCH v2 1/6] fs/writeback: avoid to writeback non-expired inode in kupdate writeback Kemeng Shi
@ 2024-02-28  9:19 ` Kemeng Shi
  2024-02-28  9:19 ` [PATCH v2 3/6] fs/writeback: remove unused parameter wb of finish_writeback_work Kemeng Shi
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Kemeng Shi @ 2024-02-28  9:19 UTC (permalink / raw
  To: viro, brauner, jack, tim.c.chen; +Cc: linux-fsdevel, linux-kernel

For case there is no more inodes for IO in io list from last wb_writeback,
We may bail out early even there is inode in dirty list should be written
back. Only bail out when we queued once to avoid missing dirtied inode.

This is from code reading...

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/fs-writeback.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 4e6166e07eaf..6fa623277d75 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -2076,6 +2076,7 @@ static long wb_writeback(struct bdi_writeback *wb,
 	struct inode *inode;
 	long progress;
 	struct blk_plug plug;
+	bool queued = false;
 
 	blk_start_plug(&plug);
 	for (;;) {
@@ -2118,8 +2119,10 @@ static long wb_writeback(struct bdi_writeback *wb,
 			dirtied_before = jiffies;
 
 		trace_writeback_start(wb, work);
-		if (list_empty(&wb->b_io))
+		if (list_empty(&wb->b_io)) {
 			queue_io(wb, work, dirtied_before);
+			queued = true;
+		}
 		if (work->sb)
 			progress = writeback_sb_inodes(work->sb, wb, work);
 		else
@@ -2142,7 +2145,7 @@ static long wb_writeback(struct bdi_writeback *wb,
 		/*
 		 * No more inodes for IO, bail
 		 */
-		if (list_empty(&wb->b_more_io)) {
+		if (list_empty(&wb->b_more_io) && queued) {
 			spin_unlock(&wb->list_lock);
 			break;
 		}
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/6] fs/writeback: remove unused parameter wb of finish_writeback_work
  2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
  2024-02-28  9:19 ` [PATCH v2 1/6] fs/writeback: avoid to writeback non-expired inode in kupdate writeback Kemeng Shi
  2024-02-28  9:19 ` [PATCH v2 2/6] fs/writeback: bail out if there is no more inodes for IO and queued once Kemeng Shi
@ 2024-02-28  9:19 ` Kemeng Shi
  2024-02-28  9:19 ` [PATCH v2 4/6] fs/writeback: only calculate dirtied_before when b_io is empty Kemeng Shi
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Kemeng Shi @ 2024-02-28  9:19 UTC (permalink / raw
  To: viro, brauner, jack, tim.c.chen; +Cc: linux-fsdevel, linux-kernel

Remove unused parameter wb of finish_writeback_work.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/fs-writeback.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 6fa623277d75..1c3134817865 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -166,8 +166,7 @@ static void wb_wakeup_delayed(struct bdi_writeback *wb)
 	spin_unlock_irq(&wb->work_lock);
 }
 
-static void finish_writeback_work(struct bdi_writeback *wb,
-				  struct wb_writeback_work *work)
+static void finish_writeback_work(struct wb_writeback_work *work)
 {
 	struct wb_completion *done = work->done;
 
@@ -196,7 +195,7 @@ static void wb_queue_work(struct bdi_writeback *wb,
 		list_add_tail(&work->list, &wb->work_list);
 		mod_delayed_work(bdi_wq, &wb->dwork, 0);
 	} else
-		finish_writeback_work(wb, work);
+		finish_writeback_work(work);
 
 	spin_unlock_irq(&wb->work_lock);
 }
@@ -2272,7 +2271,7 @@ static long wb_do_writeback(struct bdi_writeback *wb)
 	while ((work = get_next_work_item(wb)) != NULL) {
 		trace_writeback_exec(wb, work);
 		wrote += wb_writeback(wb, work);
-		finish_writeback_work(wb, work);
+		finish_writeback_work(work);
 	}
 
 	/*
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/6] fs/writeback: only calculate dirtied_before when b_io is empty
  2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
                   ` (2 preceding siblings ...)
  2024-02-28  9:19 ` [PATCH v2 3/6] fs/writeback: remove unused parameter wb of finish_writeback_work Kemeng Shi
@ 2024-02-28  9:19 ` Kemeng Shi
  2024-03-18 17:10   ` Jan Kara
  2024-02-28  9:19 ` [PATCH v2 5/6] fs/writeback: correct comment of __wakeup_flusher_threads_bdi Kemeng Shi
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Kemeng Shi @ 2024-02-28  9:19 UTC (permalink / raw
  To: viro, brauner, jack, tim.c.chen; +Cc: linux-fsdevel, linux-kernel

The dirtied_before is only used when b_io is not empty, so only calculate
when b_io is not empty.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/fs-writeback.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 1c3134817865..c98684e9e6ba 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -2105,20 +2105,21 @@ static long wb_writeback(struct bdi_writeback *wb,
 
 		spin_lock(&wb->list_lock);
 
-		/*
-		 * Kupdate and background works are special and we want to
-		 * include all inodes that need writing. Livelock avoidance is
-		 * handled by these works yielding to any other work so we are
-		 * safe.
-		 */
-		if (work->for_kupdate) {
-			dirtied_before = jiffies -
-				msecs_to_jiffies(dirty_expire_interval * 10);
-		} else if (work->for_background)
-			dirtied_before = jiffies;
-
 		trace_writeback_start(wb, work);
 		if (list_empty(&wb->b_io)) {
+			/*
+			 * Kupdate and background works are special and we want
+			 * to include all inodes that need writing. Livelock
+			 * avoidance is handled by these works yielding to any
+			 * other work so we are safe.
+			 */
+			if (work->for_kupdate) {
+				dirtied_before = jiffies -
+					msecs_to_jiffies(dirty_expire_interval *
+							 10);
+			} else if (work->for_background)
+				dirtied_before = jiffies;
+
 			queue_io(wb, work, dirtied_before);
 			queued = true;
 		}
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/6] fs/writeback: correct comment of __wakeup_flusher_threads_bdi
  2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
                   ` (3 preceding siblings ...)
  2024-02-28  9:19 ` [PATCH v2 4/6] fs/writeback: only calculate dirtied_before when b_io is empty Kemeng Shi
@ 2024-02-28  9:19 ` Kemeng Shi
  2024-02-28  9:19 ` [PATCH v2 6/6] fs/writeback: remove unnecessary return in writeback_inodes_sb Kemeng Shi
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Kemeng Shi @ 2024-02-28  9:19 UTC (permalink / raw
  To: viro, brauner, jack, tim.c.chen; +Cc: linux-fsdevel, linux-kernel

Commit e8e8a0c6c9bfc ("writeback: move nr_pages == 0 logic to one
location") removed parameter nr_pages of __wakeup_flusher_threads_bdi
and we try to writeback all dirty pages in __wakeup_flusher_threads_bdi
now. Just correct stale comment.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/fs-writeback.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index c98684e9e6ba..7e344a4e727e 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -2332,8 +2332,7 @@ void wb_workfn(struct work_struct *work)
 }
 
 /*
- * Start writeback of `nr_pages' pages on this bdi. If `nr_pages' is zero,
- * write back the whole world.
+ * Start writeback of all dirty pages on this bdi.
  */
 static void __wakeup_flusher_threads_bdi(struct backing_dev_info *bdi,
 					 enum wb_reason reason)
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 6/6] fs/writeback: remove unnecessary return in writeback_inodes_sb
  2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
                   ` (4 preceding siblings ...)
  2024-02-28  9:19 ` [PATCH v2 5/6] fs/writeback: correct comment of __wakeup_flusher_threads_bdi Kemeng Shi
@ 2024-02-28  9:19 ` Kemeng Shi
  2024-03-18 17:12 ` [PATCH v2 0/6] Fixes and cleanups to fs-writeback Jan Kara
  2024-03-19 15:18 ` Christian Brauner
  7 siblings, 0 replies; 12+ messages in thread
From: Kemeng Shi @ 2024-02-28  9:19 UTC (permalink / raw
  To: viro, brauner, jack, tim.c.chen; +Cc: linux-fsdevel, linux-kernel

writeback_inodes_sb doesn't have return value, just remove unnecessary
return in it.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/fs-writeback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 7e344a4e727e..362a5409f92e 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -2735,7 +2735,7 @@ EXPORT_SYMBOL(writeback_inodes_sb_nr);
  */
 void writeback_inodes_sb(struct super_block *sb, enum wb_reason reason)
 {
-	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages(), reason);
+	writeback_inodes_sb_nr(sb, get_nr_dirty_pages(), reason);
 }
 EXPORT_SYMBOL(writeback_inodes_sb);
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/6] fs/writeback: avoid to writeback non-expired inode in kupdate writeback
  2024-02-28  9:19 ` [PATCH v2 1/6] fs/writeback: avoid to writeback non-expired inode in kupdate writeback Kemeng Shi
@ 2024-03-18 17:07   ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2024-03-18 17:07 UTC (permalink / raw
  To: Kemeng Shi; +Cc: viro, brauner, jack, tim.c.chen, linux-fsdevel, linux-kernel

On Wed 28-02-24 17:19:53, Kemeng Shi wrote:
> In kupdate writeback, only expired inode (have been dirty for longer than
> dirty_expire_interval) is supposed to be written back. However, kupdate
> writeback will writeback non-expired inode left in b_io or b_more_io from
> last wb_writeback. As a result, writeback will keep being triggered
> unexpected when we keep dirtying pages even dirty memory is under
> threshold and inode is not expired. To be more specific:
> Assume dirty background threshold is > 1G and dirty_expire_centisecs is
> > 60s. When we running fio -size=1G -invalidate=0 -ioengine=libaio
> --time_based -runtime=60... (keep dirtying), the writeback will keep
> being triggered as following:
> wb_workfn
>   wb_do_writeback
>     wb_check_background_flush
>       /*
>        * Wb dirty background threshold starts at 0 if device was idle and
>        * grows up when bandwidth of wb is updated. So a background
>        * writeback is triggered.
>        */
>       wb_over_bg_thresh
>       /*
>        * Dirtied inode will be written back and added to b_more_io list
>        * after slice used up (because we keep dirtying the inode).
>        */
>       wb_writeback
> 
> Writeback is triggered per dirty_writeback_centisecs as following:
> wb_workfn
>   wb_do_writeback
>     wb_check_old_data_flush
>       /*
>        * Write back inode left in b_io and b_more_io from last wb_writeback
>        * even the inode is non-expired and it will be added to b_more_io
>        * again as slice will be used up (because we keep dirtying the
>        * inode)
>        */
>       wb_writeback
> 
> Fix this by moving non-expired inode to dirty list instead of more io
> list for kupdate writeback in requeue_inode.
> 
> Test as following:
> /* make it more easier to observe the issue */
> echo 300000 > /proc/sys/vm/dirty_expire_centisecs
> echo 100 > /proc/sys/vm/dirty_writeback_centisecs
> /* create a idle device */
> mkfs.ext4 -F /dev/vdb
> mount /dev/vdb /bdi1/
> /* run buffer write with fio */
> fio -name test -filename=/bdi1/file -size=800M -ioengine=libaio -bs=4K \
> -iodepth=1 -rw=write -direct=0 --time_based -runtime=60 -invalidate=0
> 
> Fio result before fix (run three tests):
> 1360MB/s
> 1329MB/s
> 1455MB/s
> 
> Fio result after fix (run three tests):
> 1737MB/s
> 1729MB/s
> 1789MB/s
> 
> Writeback for non-expired inode is gone as expeted. Observe this with trace
> writeback_start and writeback_written as following:
> echo 1 > /sys/kernel/debug/tracing/events/writeback/writeback_start/enab
> echo 1 > /sys/kernel/debug/tracing/events/writeback/writeback_written/enable
> cat /sys/kernel/tracing/trace_pipe
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/fs-writeback.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 5ab1aaf805f7..4e6166e07eaf 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1561,7 +1561,8 @@ static void inode_sleep_on_writeback(struct inode *inode)
>   * thread's back can have unexpected consequences.
>   */
>  static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
> -			  struct writeback_control *wbc)
> +			  struct writeback_control *wbc,
> +			  unsigned long dirtied_before)
>  {
>  	if (inode->i_state & I_FREEING)
>  		return;
> @@ -1594,7 +1595,8 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
>  		 * We didn't write back all the pages.  nfs_writepages()
>  		 * sometimes bales out without doing anything.
>  		 */
> -		if (wbc->nr_to_write <= 0) {
> +		if (wbc->nr_to_write <= 0 &&
> +		    !inode_dirtied_after(inode, dirtied_before)) {
>  			/* Slice used up. Queue for next turn. */
>  			requeue_io(inode, wb);
>  		} else {
> @@ -1862,6 +1864,11 @@ static long writeback_sb_inodes(struct super_block *sb,
>  	unsigned long start_time = jiffies;
>  	long write_chunk;
>  	long total_wrote = 0;  /* count both pages and inodes */
> +	unsigned long dirtied_before = jiffies;
> +
> +	if (work->for_kupdate)
> +		dirtied_before = jiffies -
> +			msecs_to_jiffies(dirty_expire_interval * 10);
>  
>  	while (!list_empty(&wb->b_io)) {
>  		struct inode *inode = wb_inode(wb->b_io.prev);
> @@ -1967,7 +1974,7 @@ static long writeback_sb_inodes(struct super_block *sb,
>  		spin_lock(&inode->i_lock);
>  		if (!(inode->i_state & I_DIRTY_ALL))
>  			total_wrote++;
> -		requeue_inode(inode, tmp_wb, &wbc);
> +		requeue_inode(inode, tmp_wb, &wbc, dirtied_before);
>  		inode_sync_complete(inode);
>  		spin_unlock(&inode->i_lock);
>  
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/6] fs/writeback: only calculate dirtied_before when b_io is empty
  2024-02-28  9:19 ` [PATCH v2 4/6] fs/writeback: only calculate dirtied_before when b_io is empty Kemeng Shi
@ 2024-03-18 17:10   ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2024-03-18 17:10 UTC (permalink / raw
  To: Kemeng Shi; +Cc: viro, brauner, jack, tim.c.chen, linux-fsdevel, linux-kernel

On Wed 28-02-24 17:19:56, Kemeng Shi wrote:
> The dirtied_before is only used when b_io is not empty, so only calculate
> when b_io is not empty.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -2105,20 +2105,21 @@ static long wb_writeback(struct bdi_writeback *wb,
>  
>  		spin_lock(&wb->list_lock);
>  
> -		/*
> -		 * Kupdate and background works are special and we want to
> -		 * include all inodes that need writing. Livelock avoidance is
> -		 * handled by these works yielding to any other work so we are
> -		 * safe.
> -		 */
> -		if (work->for_kupdate) {
> -			dirtied_before = jiffies -
> -				msecs_to_jiffies(dirty_expire_interval * 10);
> -		} else if (work->for_background)
> -			dirtied_before = jiffies;
> -
>  		trace_writeback_start(wb, work);
>  		if (list_empty(&wb->b_io)) {
> +			/*
> +			 * Kupdate and background works are special and we want
> +			 * to include all inodes that need writing. Livelock
> +			 * avoidance is handled by these works yielding to any
> +			 * other work so we are safe.
> +			 */
> +			if (work->for_kupdate) {
> +				dirtied_before = jiffies -
> +					msecs_to_jiffies(dirty_expire_interval *
> +							 10);
> +			} else if (work->for_background)
> +				dirtied_before = jiffies;
> +
>  			queue_io(wb, work, dirtied_before);
>  			queued = true;
>  		}
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/6] Fixes and cleanups to fs-writeback
  2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
                   ` (5 preceding siblings ...)
  2024-02-28  9:19 ` [PATCH v2 6/6] fs/writeback: remove unnecessary return in writeback_inodes_sb Kemeng Shi
@ 2024-03-18 17:12 ` Jan Kara
  2024-03-19 15:19   ` Christian Brauner
  2024-03-19 15:18 ` Christian Brauner
  7 siblings, 1 reply; 12+ messages in thread
From: Jan Kara @ 2024-03-18 17:12 UTC (permalink / raw
  To: Christian Brauner
  Cc: viro, jack, Kemeng Shi, tim.c.chen, linux-fsdevel, linux-kernel

On Wed 28-02-24 17:19:52, Kemeng Shi wrote:
> v1->v2:
> -Filter non-expired in requeue_inode in patch "fs/writeback: avoid to
> writeback non-expired inode in kupdate writeback"
> -Wrap the comment at 80 columns in patch "fs/writeback: only calculate
> dirtied_before when b_io is empty"
> -Abandon patch "fs/writeback: remove unneeded check in
> writeback_single_inode"
> -Collect RVB from Jan and Tim

Christian, the series looks good to me. Please pick it up once your tree
settles after the merge window. Thanks!

								Honza

> 
> Kemeng Shi (6):
>   fs/writeback: avoid to writeback non-expired inode in kupdate
>     writeback
>   fs/writeback: bail out if there is no more inodes for IO and queued
>     once
>   fs/writeback: remove unused parameter wb of finish_writeback_work
>   fs/writeback: only calculate dirtied_before when b_io is empty
>   fs/writeback: correct comment of __wakeup_flusher_threads_bdi
>   fs/writeback: remove unnecessary return in writeback_inodes_sb
> 
>  fs/fs-writeback.c | 57 +++++++++++++++++++++++++++--------------------
>  1 file changed, 33 insertions(+), 24 deletions(-)
> 
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/6] Fixes and cleanups to fs-writeback
  2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
                   ` (6 preceding siblings ...)
  2024-03-18 17:12 ` [PATCH v2 0/6] Fixes and cleanups to fs-writeback Jan Kara
@ 2024-03-19 15:18 ` Christian Brauner
  7 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2024-03-19 15:18 UTC (permalink / raw
  To: Kemeng Shi
  Cc: Christian Brauner, linux-fsdevel, linux-kernel, tim.c.chen, viro,
	jack

On Wed, 28 Feb 2024 17:19:52 +0800, Kemeng Shi wrote:
> v1->v2:
> -Filter non-expired in requeue_inode in patch "fs/writeback: avoid to
> writeback non-expired inode in kupdate writeback"
> -Wrap the comment at 80 columns in patch "fs/writeback: only calculate
> dirtied_before when b_io is empty"
> -Abandon patch "fs/writeback: remove unneeded check in
> writeback_single_inode"
> -Collect RVB from Jan and Tim
> 
> [...]

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc

[1/6] fs/writeback: avoid to writeback non-expired inode in kupdate writeback
      https://git.kernel.org/vfs/vfs/c/c66cf7bdd77c
[2/6] fs/writeback: bail out if there is no more inodes for IO and queued once
      https://git.kernel.org/vfs/vfs/c/d82e51471fc3
[3/6] fs/writeback: remove unused parameter wb of finish_writeback_work
      https://git.kernel.org/vfs/vfs/c/7cb6d20fc517
[4/6] fs/writeback: only calculate dirtied_before when b_io is empty
      https://git.kernel.org/vfs/vfs/c/e5cb59d053c2
[5/6] fs/writeback: correct comment of __wakeup_flusher_threads_bdi
      https://git.kernel.org/vfs/vfs/c/78f2b24980d8
[6/6] fs/writeback: remove unnecessary return in writeback_inodes_sb
      https://git.kernel.org/vfs/vfs/c/ed9d128c0c42

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/6] Fixes and cleanups to fs-writeback
  2024-03-18 17:12 ` [PATCH v2 0/6] Fixes and cleanups to fs-writeback Jan Kara
@ 2024-03-19 15:19   ` Christian Brauner
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2024-03-19 15:19 UTC (permalink / raw
  To: Jan Kara; +Cc: viro, Kemeng Shi, tim.c.chen, linux-fsdevel, linux-kernel

> Christian, the series looks good to me. Please pick it up once your tree
> settles after the merge window. Thanks!

Thanks for the heads-up, Jan! I've picked it now so it isn't forgotten
and will rebase once -rc1 is out.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-03-19 15:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-28  9:19 [PATCH v2 0/6] Fixes and cleanups to fs-writeback Kemeng Shi
2024-02-28  9:19 ` [PATCH v2 1/6] fs/writeback: avoid to writeback non-expired inode in kupdate writeback Kemeng Shi
2024-03-18 17:07   ` Jan Kara
2024-02-28  9:19 ` [PATCH v2 2/6] fs/writeback: bail out if there is no more inodes for IO and queued once Kemeng Shi
2024-02-28  9:19 ` [PATCH v2 3/6] fs/writeback: remove unused parameter wb of finish_writeback_work Kemeng Shi
2024-02-28  9:19 ` [PATCH v2 4/6] fs/writeback: only calculate dirtied_before when b_io is empty Kemeng Shi
2024-03-18 17:10   ` Jan Kara
2024-02-28  9:19 ` [PATCH v2 5/6] fs/writeback: correct comment of __wakeup_flusher_threads_bdi Kemeng Shi
2024-02-28  9:19 ` [PATCH v2 6/6] fs/writeback: remove unnecessary return in writeback_inodes_sb Kemeng Shi
2024-03-18 17:12 ` [PATCH v2 0/6] Fixes and cleanups to fs-writeback Jan Kara
2024-03-19 15:19   ` Christian Brauner
2024-03-19 15:18 ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).