[PATCH v2 1/2] writeback: Wake up waiting tasks when finishing the writeback of a chunk.

[PATCH v2 1/2] writeback: Wake up waiting tasks when finishing the writeback of a chunk.

[Julian Sun]

Writing back a large number of pages can take a lots of time.
This issue is exacerbated when the underlying device is slow or
subject to block layer rate limiting, which in turn triggers
unexpected hung task warnings.

We can trigger a wake-up once a chunk has been written back and the
waiting time for writeback exceeds half of
sysctl_hung_task_timeout_secs.
This action allows the hung task detector to be aware of the writeback
progress, thereby eliminating these unexpected hung task warnings.

This patch has passed the xfstests 'check -g quick' test based on ext4,
with no additional failures introduced.

Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
---
 Changes in v2:
  * remove code in finish_writeback_work()
  * rename stamp to progress_stamp
  * only report progress if there's any task waiting

 fs/fs-writeback.c                | 11 ++++++++++-
 include/linux/backing-dev-defs.h |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index a07b8cf73ae2..61785a9d6669 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -14,6 +14,7 @@
  *		Additions for address_space-based writeback
  */
 
+#include <linux/sched/sysctl.h>
 #include <linux/kernel.h>
 #include <linux/export.h>
 #include <linux/spinlock.h>
@@ -213,7 +214,8 @@ static void wb_queue_work(struct bdi_writeback *wb,
 void wb_wait_for_completion(struct wb_completion *done)
 {
 	atomic_dec(&done->cnt);		/* put down the initial count */
-	wait_event(*done->waitq, !atomic_read(&done->cnt));
+	wait_event(*done->waitq,
+		   ({ done->progress_stamp = jiffies; !atomic_read(&done->cnt); }));
 }
 
 #ifdef CONFIG_CGROUP_WRITEBACK
@@ -1893,6 +1895,7 @@ static long writeback_sb_inodes(struct super_block *sb,
 	long write_chunk;
 	long total_wrote = 0;  /* count both pages and inodes */
 	unsigned long dirtied_before = jiffies;
+	unsigned long progress_stamp;
 
 	if (work->for_kupdate)
 		dirtied_before = jiffies -
@@ -1975,6 +1978,12 @@ static long writeback_sb_inodes(struct super_block *sb,
 		 */
 		__writeback_single_inode(inode, &wbc);
 
+		/* Report progress to inform the hung task detector of the progress. */
+		progress_stamp = work->done->progress_stamp;
+		if (work->done && progress_stamp && (jiffies - progress_stamp) >
+		    HZ * sysctl_hung_task_timeout_secs / 2)
+			wake_up_all(work->done->waitq);
+
 		wbc_detach_inode(&wbc);
 		work->nr_pages -= write_chunk - wbc.nr_to_write;
 		wrote = write_chunk - wbc.nr_to_write - wbc.pages_skipped;
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 2ad261082bba..1057060bb2aa 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -63,6 +63,7 @@ enum wb_reason {
 struct wb_completion {
 	atomic_t		cnt;
 	wait_queue_head_t	*waitq;
+	unsigned long progress_stamp;	/* The jiffies when slow progress is detected */
 };
 
 #define __WB_COMPLETION_INIT(_waitq)	\
-- 
2.39.5

[PATCH v2 2/2] writeback: Add logging for slow writeback (exceeds sysctl_hung_task_timeout_secs)

[Julian Sun]

When a writeback work lasts for sysctl_hung_task_timeout_secs, we want
to identify that it's slow-this helps us pinpoint potential issues.

Additionally, recording the starting jiffies is useful when debugging a
crashed vmcore.

Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
 Changes in v2:
  * rename start to wait_stamp
  * init wait_stamp in wb_wait_for_completion()

 fs/fs-writeback.c                | 11 ++++++++++-
 include/linux/backing-dev-defs.h |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 61785a9d6669..131d0d11672b 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -213,6 +213,7 @@ static void wb_queue_work(struct bdi_writeback *wb,
  */
 void wb_wait_for_completion(struct wb_completion *done)
 {
+	done->wait_stamp = jiffies;
 	atomic_dec(&done->cnt);		/* put down the initial count */
 	wait_event(*done->waitq,
 		   ({ done->progress_stamp = jiffies; !atomic_read(&done->cnt); }));
@@ -1981,8 +1982,16 @@ static long writeback_sb_inodes(struct super_block *sb,
 		/* Report progress to inform the hung task detector of the progress. */
 		progress_stamp = work->done->progress_stamp;
 		if (work->done && progress_stamp && (jiffies - progress_stamp) >
-		    HZ * sysctl_hung_task_timeout_secs / 2)
+		    HZ * sysctl_hung_task_timeout_secs / 2) {
+			unsigned long wait_secs = (jiffies - work->done->wait_stamp) / HZ;
+
+			if (wait_secs >= sysctl_hung_task_timeout_secs)
+				pr_info("The writeback work for bdi(%s) has lasted "
+					"for more than %lu seconds with agv_bw %ld\n",
+					wb->bdi->dev_name, wait_secs,
+					READ_ONCE(wb->avg_write_bandwidth));
 			wake_up_all(work->done->waitq);
+		}
 
 		wbc_detach_inode(&wbc);
 		work->nr_pages -= write_chunk - wbc.nr_to_write;
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 1057060bb2aa..a92a223bdf3d 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -64,6 +64,7 @@ struct wb_completion {
 	atomic_t		cnt;
 	wait_queue_head_t	*waitq;
 	unsigned long progress_stamp;	/* The jiffies when slow progress is detected */
+	unsigned long wait_stamp; /* The jiffies when waiting for the writeback work to finish */
 };
 
 #define __WB_COMPLETION_INIT(_waitq)	\
-- 
2.39.5

Re: [PATCH v2 1/2] writeback: Wake up waiting tasks when finishing the writeback of a chunk.

[Jan Kara]

On Mon 29-09-25 20:28:49, Julian Sun wrote:
> Writing back a large number of pages can take a lots of time.
> This issue is exacerbated when the underlying device is slow or
> subject to block layer rate limiting, which in turn triggers
> unexpected hung task warnings.
> 
> We can trigger a wake-up once a chunk has been written back and the
> waiting time for writeback exceeds half of
> sysctl_hung_task_timeout_secs.
> This action allows the hung task detector to be aware of the writeback
> progress, thereby eliminating these unexpected hung task warnings.
> 
> This patch has passed the xfstests 'check -g quick' test based on ext4,
> with no additional failures introduced.
> 
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>

Looks good. Feel free to add:

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

								Honza

> ---
>  Changes in v2:
>   * remove code in finish_writeback_work()
>   * rename stamp to progress_stamp
>   * only report progress if there's any task waiting
> 
>  fs/fs-writeback.c                | 11 ++++++++++-
>  include/linux/backing-dev-defs.h |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index a07b8cf73ae2..61785a9d6669 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -14,6 +14,7 @@
>   *		Additions for address_space-based writeback
>   */
>  
> +#include <linux/sched/sysctl.h>
>  #include <linux/kernel.h>
>  #include <linux/export.h>
>  #include <linux/spinlock.h>
> @@ -213,7 +214,8 @@ static void wb_queue_work(struct bdi_writeback *wb,
>  void wb_wait_for_completion(struct wb_completion *done)
>  {
>  	atomic_dec(&done->cnt);		/* put down the initial count */
> -	wait_event(*done->waitq, !atomic_read(&done->cnt));
> +	wait_event(*done->waitq,
> +		   ({ done->progress_stamp = jiffies; !atomic_read(&done->cnt); }));
>  }
>  
>  #ifdef CONFIG_CGROUP_WRITEBACK
> @@ -1893,6 +1895,7 @@ static long writeback_sb_inodes(struct super_block *sb,
>  	long write_chunk;
>  	long total_wrote = 0;  /* count both pages and inodes */
>  	unsigned long dirtied_before = jiffies;
> +	unsigned long progress_stamp;
>  
>  	if (work->for_kupdate)
>  		dirtied_before = jiffies -
> @@ -1975,6 +1978,12 @@ static long writeback_sb_inodes(struct super_block *sb,
>  		 */
>  		__writeback_single_inode(inode, &wbc);
>  
> +		/* Report progress to inform the hung task detector of the progress. */
> +		progress_stamp = work->done->progress_stamp;
> +		if (work->done && progress_stamp && (jiffies - progress_stamp) >
> +		    HZ * sysctl_hung_task_timeout_secs / 2)
> +			wake_up_all(work->done->waitq);
> +
>  		wbc_detach_inode(&wbc);
>  		work->nr_pages -= write_chunk - wbc.nr_to_write;
>  		wrote = write_chunk - wbc.nr_to_write - wbc.pages_skipped;
> diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
> index 2ad261082bba..1057060bb2aa 100644
> --- a/include/linux/backing-dev-defs.h
> +++ b/include/linux/backing-dev-defs.h
> @@ -63,6 +63,7 @@ enum wb_reason {
>  struct wb_completion {
>  	atomic_t		cnt;
>  	wait_queue_head_t	*waitq;
> +	unsigned long progress_stamp;	/* The jiffies when slow progress is detected */
>  };
>  
>  #define __WB_COMPLETION_INIT(_waitq)	\
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2 2/2] writeback: Add logging for slow writeback (exceeds sysctl_hung_task_timeout_secs)

[Jan Kara]

On Mon 29-09-25 20:28:50, Julian Sun wrote:
> When a writeback work lasts for sysctl_hung_task_timeout_secs, we want
> to identify that it's slow-this helps us pinpoint potential issues.
> 
> Additionally, recording the starting jiffies is useful when debugging a
> crashed vmcore.
> 
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>

So this works but I'd rather do:

> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 61785a9d6669..131d0d11672b 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -213,6 +213,7 @@ static void wb_queue_work(struct bdi_writeback *wb,
>   */
>  void wb_wait_for_completion(struct wb_completion *done)
>  {
> +	done->wait_stamp = jiffies;
>  	atomic_dec(&done->cnt);		/* put down the initial count */
>  	wait_event(*done->waitq,
>  		   ({ done->progress_stamp = jiffies; !atomic_read(&done->cnt); }));

static bool wb_wait_for_completion_cb(struct wb_completion *done,
				      unsigned long wait_start)
{
	done->progress_stamp = jiffies;
	if ((jiffies - wait_start) / HZ > sysctl_hung_task_timeout_secs)
		pr_info(dump here kind of hang check warning);

	return !atomic_read(&done->cnt);
}

void wb_wait_for_completion(struct wb_completion *done)
{
	unsigned long wait_start = jiffies;

	atomic_dec(&done->cnt);		/* put down the initial count */
	wait_event(*done->waitq, wb_wait_for_completion_cb(done, wait_start));
}

This way we can properly dump info about blocked task which is IMO more
interesting than info about BDI.

								Honza

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

Re: [External] Re: [PATCH v2 2/2] writeback: Add logging for slow writeback (exceeds sysctl_hung_task_timeout_secs)

[Julian Sun]

On Mon, Sep 29, 2025 at 11:51 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 29-09-25 20:28:50, Julian Sun wrote:
> > When a writeback work lasts for sysctl_hung_task_timeout_secs, we want
> > to identify that it's slow-this helps us pinpoint potential issues.
> >
> > Additionally, recording the starting jiffies is useful when debugging a
> > crashed vmcore.
> >
> > Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
>
> So this works but I'd rather do:
>
> > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > index 61785a9d6669..131d0d11672b 100644
> > --- a/fs/fs-writeback.c
> > +++ b/fs/fs-writeback.c
> > @@ -213,6 +213,7 @@ static void wb_queue_work(struct bdi_writeback *wb,
> >   */
> >  void wb_wait_for_completion(struct wb_completion *done)
> >  {
> > +     done->wait_stamp = jiffies;
> >       atomic_dec(&done->cnt);         /* put down the initial count */
> >       wait_event(*done->waitq,
> >                  ({ done->progress_stamp = jiffies; !atomic_read(&done->cnt); }));
>
> static bool wb_wait_for_completion_cb(struct wb_completion *done,
>                                       unsigned long wait_start)
> {
>         done->progress_stamp = jiffies;
>         if ((jiffies - wait_start) / HZ > sysctl_hung_task_timeout_secs)
>                 pr_info(dump here kind of hang check warning);
>
>         return !atomic_read(&done->cnt);
> }
>
> void wb_wait_for_completion(struct wb_completion *done)
> {
>         unsigned long wait_start = jiffies;
>
>         atomic_dec(&done->cnt);         /* put down the initial count */
>         wait_event(*done->waitq, wb_wait_for_completion_cb(done, wait_start));
> }
>
> This way we can properly dump info about blocked task which is IMO more
> interesting than info about BDI.

Indeed, this makes more sense. Thank you for your suggestion.
>
>                                                                 Honza
>
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR



-- 
Julian Sun <sunjunchao@bytedance.com>