mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Suppress undesirable hung task warnings.
@ 2025-09-24  3:40 Julian Sun
  2025-09-24  3:40 ` [PATCH v2 1/2] hung_task: Introduce touch_hung_task_detector() Julian Sun
  2025-09-24  3:41 ` [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources Julian Sun
  0 siblings, 2 replies; 8+ messages in thread
From: Julian Sun @ 2025-09-24  3:40 UTC (permalink / raw)
  To: cgroups, linux-kernel
  Cc: akpm, lance.yang, mhiramat, yangyicong, will, dianders, mingo,
	lihuafei1, hannes, mhocko, roman.gushchin, shakeel.butt,
	muchun.song, tj, peterz

As suggested by Andrew Morton in [1], we need a general mechanism 
that allows the hung task detector to ignore unnecessary hung 
tasks. This patch set implements this functionality and enables it
in memcg.

Patch 1 introduces touch_hung_task_detector(), which allows a task to 
mark itself and then hung task detector will ignore warnings for it.

Patch 2 uses touch_hung_task_detector() in the final phase of memcg 
teardown to eliminate the hung task warning.

[1]: https://lore.kernel.org/all/20250917152155.5a8ddb3e4ff813289ea0b4c9@linux-foundation.org/

Julian Sun (2):
  hung_task: Introduce touch_hung_task_dector().
  memcg: Don't trigger hung task warnings when memcg is releasing
    resources.

 include/linux/nmi.h |  2 ++
 kernel/hung_task.c  | 13 +++++++++++++
 mm/memcontrol.c     |  5 ++++-
 3 files changed, 19 insertions(+), 1 deletion(-)

-- 
2.39.5


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

* [PATCH v2 1/2] hung_task: Introduce touch_hung_task_detector().
  2025-09-24  3:40 [PATCH v2 0/2] Suppress undesirable hung task warnings Julian Sun
@ 2025-09-24  3:40 ` Julian Sun
  2025-09-25 13:33   ` Masami Hiramatsu
  2025-09-24  3:41 ` [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources Julian Sun
  1 sibling, 1 reply; 8+ messages in thread
From: Julian Sun @ 2025-09-24  3:40 UTC (permalink / raw)
  To: cgroups, linux-kernel
  Cc: akpm, lance.yang, mhiramat, yangyicong, will, dianders, mingo,
	lihuafei1, hannes, mhocko, roman.gushchin, shakeel.butt,
	muchun.song, tj, peterz

In the kernel, long waits can trigger hung task warnings. However, some
warnings are undesirable and unnecessary - for example, a hung task
warning triggered when a background kworker waits for writeback
completion during resource cleanup(like the context of
mem_cgroup_css_free()). This kworker does not affect any user behavior
and there is no erroneous behavior at the kernel code level, yet it
triggers an annoying hung task warning.

To eliminate such warnings, this patch introduces
touch_hung_task_detector() to allow some tasks ignored by hung task
detector.

Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Suggested-by: Lance Yang <lance.yang@linux.dev>
---
 include/linux/nmi.h |  2 ++
 kernel/hung_task.c  | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index cf3c6ab408aa..61fc2ad234de 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -59,8 +59,10 @@ static inline void touch_all_softlockup_watchdogs(void) { }
 
 #ifdef CONFIG_DETECT_HUNG_TASK
 void reset_hung_task_detector(void);
+void touch_hung_task_detector(struct task_struct *t);
 #else
 static inline void reset_hung_task_detector(void) { }
+static inline void touch_hung_task_detector(struct task_struct *t) { }
 #endif
 
 /*
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 8708a1205f82..6409d3d4bd36 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -184,6 +184,11 @@ static inline void debug_show_blocker(struct task_struct *task)
 }
 #endif
 
+void touch_hung_task_detector(struct task_struct *t)
+{
+	t->last_switch_count = ULONG_MAX;
+}
+
 static void check_hung_task(struct task_struct *t, unsigned long timeout)
 {
 	unsigned long switch_count = t->nvcsw + t->nivcsw;
@@ -203,6 +208,10 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
 	if (unlikely(!switch_count))
 		return;
 
+	/* The task doesn't want to trigger the hung task warning. */
+	if (unlikely(t->last_switch_count == ULONG_MAX))
+		return;
+
 	if (switch_count != t->last_switch_count) {
 		t->last_switch_count = switch_count;
 		t->last_switch_time = jiffies;
@@ -317,6 +326,10 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
 		    !(state & TASK_WAKEKILL) &&
 		    !(state & TASK_NOLOAD))
 			check_hung_task(t, timeout);
+		else if (unlikely(t->last_switch_count == ULONG_MAX)) {
+			t->last_switch_count = t->nvcsw + t->nivcsw;
+			t->last_switch_time = jiffies;
+		}
 	}
  unlock:
 	rcu_read_unlock();
-- 
2.39.5


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

* [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources.
  2025-09-24  3:40 [PATCH v2 0/2] Suppress undesirable hung task warnings Julian Sun
  2025-09-24  3:40 ` [PATCH v2 1/2] hung_task: Introduce touch_hung_task_detector() Julian Sun
@ 2025-09-24  3:41 ` Julian Sun
  2025-09-24  6:32   ` Peter Zijlstra
  1 sibling, 1 reply; 8+ messages in thread
From: Julian Sun @ 2025-09-24  3:41 UTC (permalink / raw)
  To: cgroups, linux-kernel
  Cc: akpm, lance.yang, mhiramat, yangyicong, will, dianders, mingo,
	lihuafei1, hannes, mhocko, roman.gushchin, shakeel.butt,
	muchun.song, tj, peterz

Hung task warning in mem_cgroup_css_free() is undesirable and
unnecessary since the behavior of waiting for a long time is
expected.

Use touch_hung_task_detector() to eliminate the possible
hung task warning.

Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---

 I didn’t add a fixes tag because there is no actual bug in the
 original code, and this patch is more of an improvement-type one.

 mm/memcontrol.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8dd7fbed5a94..fc73a56372a4 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -63,6 +63,7 @@
 #include <linux/seq_buf.h>
 #include <linux/sched/isolation.h>
 #include <linux/kmemleak.h>
+#include <linux/nmi.h>
 #include "internal.h"
 #include <net/sock.h>
 #include <net/ip.h>
@@ -3912,8 +3913,15 @@ static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
 	int __maybe_unused i;
 
 #ifdef CONFIG_CGROUP_WRITEBACK
-	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++)
+	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++) {
+		/*
+		 * We don't want the hung task detector to report warnings
+		 * here since there's nothing wrong if the writeback work
+		 * lasts for a long time.
+		 */
+		touch_hung_task_detector(current);
 		wb_wait_for_completion(&memcg->cgwb_frn[i].done);
+	}
 #endif
 	if (cgroup_subsys_on_dfl(memory_cgrp_subsys) && !cgroup_memory_nosocket)
 		static_branch_dec(&memcg_sockets_enabled_key);
-- 
2.39.5


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

* Re: [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources.
  2025-09-24  3:41 ` [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources Julian Sun
@ 2025-09-24  6:32   ` Peter Zijlstra
  2025-09-24  7:50     ` [External] " Julian Sun
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2025-09-24  6:32 UTC (permalink / raw)
  To: Julian Sun
  Cc: cgroups, linux-kernel, akpm, lance.yang, mhiramat, yangyicong,
	will, dianders, mingo, lihuafei1, hannes, mhocko, roman.gushchin,
	shakeel.butt, muchun.song, tj

On Wed, Sep 24, 2025 at 11:41:00AM +0800, Julian Sun wrote:
> Hung task warning in mem_cgroup_css_free() is undesirable and
> unnecessary since the behavior of waiting for a long time is
> expected.
> 
> Use touch_hung_task_detector() to eliminate the possible
> hung task warning.
> 
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>

Still hate this. It is not tied to progress. If progress really stalls,
no warning will be given.

>  mm/memcontrol.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 8dd7fbed5a94..fc73a56372a4 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -63,6 +63,7 @@
>  #include <linux/seq_buf.h>
>  #include <linux/sched/isolation.h>
>  #include <linux/kmemleak.h>
> +#include <linux/nmi.h>
>  #include "internal.h"
>  #include <net/sock.h>
>  #include <net/ip.h>
> @@ -3912,8 +3913,15 @@ static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
>  	int __maybe_unused i;
>  
>  #ifdef CONFIG_CGROUP_WRITEBACK
> -	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++)
> +	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++) {
> +		/*
> +		 * We don't want the hung task detector to report warnings
> +		 * here since there's nothing wrong if the writeback work
> +		 * lasts for a long time.
> +		 */
> +		touch_hung_task_detector(current);
>  		wb_wait_for_completion(&memcg->cgwb_frn[i].done);
> +	}
>  #endif
>  	if (cgroup_subsys_on_dfl(memory_cgrp_subsys) && !cgroup_memory_nosocket)
>  		static_branch_dec(&memcg_sockets_enabled_key);
> -- 
> 2.39.5
> 

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

* Re: [External] Re: [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources.
  2025-09-24  6:32   ` Peter Zijlstra
@ 2025-09-24  7:50     ` Julian Sun
  2025-09-24  8:28       ` Peter Zijlstra
  0 siblings, 1 reply; 8+ messages in thread
From: Julian Sun @ 2025-09-24  7:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: cgroups, linux-kernel, akpm, lance.yang, mhiramat, yangyicong,
	will, dianders, mingo, lihuafei1, hannes, mhocko, roman.gushchin,
	shakeel.butt, muchun.song, tj

On 9/24/25 2:32 PM, Peter Zijlstra wrote:
> On Wed, Sep 24, 2025 at 11:41:00AM +0800, Julian Sun wrote:
>> Hung task warning in mem_cgroup_css_free() is undesirable and
>> unnecessary since the behavior of waiting for a long time is
>> expected.
>>
>> Use touch_hung_task_detector() to eliminate the possible
>> hung task warning.
>>
>> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
> 
> Still hate this. It is not tied to progress. If progress really stalls,
> no warning will be given.

Hi, peter

Thanks for your review and comments.

I did take a look at your solution provided yesterday, and get your 
point. However AFAICS it can't resolve the unexpected warnings here. 
Because it only works after we reach the finish_writeback_work(), and 
the key point here is, it *already* takes a long time before we reach 
finish_writeback_work(), and there is true progress before finish the 
writeback work that hung task detector still can not know.

If we want to make the hung task detector to known the progress of 
writeback work, we need to add some code within do_writepages(): after 
each finish of a_ops->writepages(), we need to make detector to known 
there's progress. Something like this:

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 3e248d1c3969..49572a83c47b 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2635,6 +2635,10 @@ int do_writepages(struct address_space *mapping, 
struct writeback_control *wbc)
                 else
                         /* deal with chardevs and other special files */
                         ret = 0;
+               /* Make hung task detector to known there's progress. */
+               if (force_wake)
+                       wake_up_all(waitq);
+
                 if (ret != -ENOMEM || wbc->sync_mode != WB_SYNC_ALL)
                         break;

which has a big impact on current code - I don't want to introduce this.

Yes, the behavior in this patch does have the possibility to paper cover 
the real warnings, and what I want to argue is that the essence of this 
patch is the same as the current touch_nmi_watchdog() and 
touch_softlockup_watchdog() - these functions are used only in specific 
scenarios we known and only affect a single event. And there seems no 
report that touch_nmi/softlockup_watchdog() will paper cover the real 
warnings (do we?).

Correct me if there's anything I'm missing or misunderstanding.


> 
>>   mm/memcontrol.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 8dd7fbed5a94..fc73a56372a4 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -63,6 +63,7 @@
>>   #include <linux/seq_buf.h>
>>   #include <linux/sched/isolation.h>
>>   #include <linux/kmemleak.h>
>> +#include <linux/nmi.h>
>>   #include "internal.h"
>>   #include <net/sock.h>
>>   #include <net/ip.h>
>> @@ -3912,8 +3913,15 @@ static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
>>   	int __maybe_unused i;
>>   
>>   #ifdef CONFIG_CGROUP_WRITEBACK
>> -	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++)
>> +	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++) {
>> +		/*
>> +		 * We don't want the hung task detector to report warnings
>> +		 * here since there's nothing wrong if the writeback work
>> +		 * lasts for a long time.
>> +		 */
>> +		touch_hung_task_detector(current);
>>   		wb_wait_for_completion(&memcg->cgwb_frn[i].done);
>> +	}
>>   #endif
>>   	if (cgroup_subsys_on_dfl(memory_cgrp_subsys) && !cgroup_memory_nosocket)
>>   		static_branch_dec(&memcg_sockets_enabled_key);
>> -- 
>> 2.39.5
>>

Thanks,
-- 
Julian Sun <sunjunchao@bytedance.com>

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

* Re: [External] Re: [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources.
  2025-09-24  7:50     ` [External] " Julian Sun
@ 2025-09-24  8:28       ` Peter Zijlstra
  2025-09-24 10:36         ` Julian Sun
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2025-09-24  8:28 UTC (permalink / raw)
  To: Julian Sun
  Cc: cgroups, linux-kernel, akpm, lance.yang, mhiramat, yangyicong,
	will, dianders, mingo, lihuafei1, hannes, mhocko, roman.gushchin,
	shakeel.butt, muchun.song, tj

On Wed, Sep 24, 2025 at 03:50:41PM +0800, Julian Sun wrote:
> On 9/24/25 2:32 PM, Peter Zijlstra wrote:
> > On Wed, Sep 24, 2025 at 11:41:00AM +0800, Julian Sun wrote:
> > > Hung task warning in mem_cgroup_css_free() is undesirable and
> > > unnecessary since the behavior of waiting for a long time is
> > > expected.
> > > 
> > > Use touch_hung_task_detector() to eliminate the possible
> > > hung task warning.
> > > 
> > > Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
> > 
> > Still hate this. It is not tied to progress. If progress really stalls,
> > no warning will be given.
> 
> Hi, peter
> 
> Thanks for your review and comments.
> 
> I did take a look at your solution provided yesterday, and get your point.
> However AFAICS it can't resolve the unexpected warnings here. Because it
> only works after we reach the finish_writeback_work(), and the key point
> here is, it *already* takes a long time before we reach
> finish_writeback_work(), and there is true progress before finish the
> writeback work that hung task detector still can not know.

But wb_split_bdi_pages() should already split things into smaller chunks
if there is low bandwidth, right? And we call finish_writeback_work()
for each chunk.

If a chunk is still taking too long, surely the solution is to use
smaller chunks?

> If we want to make the hung task detector to known the progress of writeback
> work, we need to add some code within do_writepages(): after each finish of
> a_ops->writepages(), we need to make detector to known there's progress.
> Something like this:
> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 3e248d1c3969..49572a83c47b 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -2635,6 +2635,10 @@ int do_writepages(struct address_space *mapping,
> struct writeback_control *wbc)
>                 else
>                         /* deal with chardevs and other special files */
>                         ret = 0;
> +               /* Make hung task detector to known there's progress. */
> +               if (force_wake)
> +                       wake_up_all(waitq);
> +
>                 if (ret != -ENOMEM || wbc->sync_mode != WB_SYNC_ALL)
>                         break;
> 
> which has a big impact on current code - I don't want to introduce this.

You sure? It looks to me like the next level down is wb_writeback() and
writeback_sb_inodes(), and those already have time based breaks in and
still have access to wb_writeback_work::done, while do_writepages() no
longer has that context.

> Yes, the behavior in this patch does have the possibility to paper cover the
> real warnings, and what I want to argue is that the essence of this patch is
> the same as the current touch_nmi_watchdog() and touch_softlockup_watchdog()
> - these functions are used only in specific scenarios we known and only
> affect a single event. And there seems no report that
> touch_nmi/softlockup_watchdog() will paper cover the real warnings (do we?).
> 
> Correct me if there's anything I'm missing or misunderstanding.

The thing with touch_nmi_watchdog() is that you need to keep doing it.
The moment you stop calling touch_nmi_watchdog(), you will cause it to
fire.

That is very much in line with the thing I proposed, and rather unlike
your proposal that blanket kill reporting for the task, irrespective of
how long it sits there waiting.


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

* Re: [External] Re: [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources.
  2025-09-24  8:28       ` Peter Zijlstra
@ 2025-09-24 10:36         ` Julian Sun
  0 siblings, 0 replies; 8+ messages in thread
From: Julian Sun @ 2025-09-24 10:36 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: cgroups, linux-kernel, akpm, lance.yang, mhiramat, yangyicong,
	will, dianders, mingo, lihuafei1, hannes, mhocko, roman.gushchin,
	shakeel.butt, muchun.song, tj, brauner

On 9/24/25 4:28 PM, Peter Zijlstra wrote:

Hi,
> On Wed, Sep 24, 2025 at 03:50:41PM +0800, Julian Sun wrote:
>> On 9/24/25 2:32 PM, Peter Zijlstra wrote:
>>> On Wed, Sep 24, 2025 at 11:41:00AM +0800, Julian Sun wrote:
>>>> Hung task warning in mem_cgroup_css_free() is undesirable and
>>>> unnecessary since the behavior of waiting for a long time is
>>>> expected.
>>>>
>>>> Use touch_hung_task_detector() to eliminate the possible
>>>> hung task warning.
>>>>
>>>> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
>>>
>>> Still hate this. It is not tied to progress. If progress really stalls,
>>> no warning will be given.
>>
>> Hi, peter
>>
>> Thanks for your review and comments.
>>
>> I did take a look at your solution provided yesterday, and get your point.
>> However AFAICS it can't resolve the unexpected warnings here. Because it
>> only works after we reach the finish_writeback_work(), and the key point
>> here is, it *already* takes a long time before we reach
>> finish_writeback_work(), and there is true progress before finish the
>> writeback work that hung task detector still can not know.
> 
> But wb_split_bdi_pages() should already split things into smaller chunks
> if there is low bandwidth, right? And we call finish_writeback_work()
> for each chunk.

AFAICS, wb_split_bdi_pages() will only be invoked in the sync scenarios, 
and not in the background writeback scenarios - which is exactly the 
case here.

And I noticed that there's something similar in background writeback, 
where writeback_chunk_size() will split all pages into several chunks, 
the min chunk size is 1024(MIN_WRITEBACK_PAGES) pages. The difference 
from wb_split_bdi_pages() is that we don't report progress after 
finishing the writeback of a chunk.
> 
> If a chunk is still taking too long, surely the solution is to use
> smaller chunks?

Yeah it still takes a long time, I checked the write_bandwidth and 
avg_write_bandwidth when warning was triggered:

	>>> wb.write_bandwidth
	(unsigned long)24
	>>> wb.avg_write_bandwidth
	(unsigned long)24
	>>> wb.write_bandwidth
	(unsigned long)13
	>>> wb.write_bandwidth
	(unsigned long)13

At this bandwidth, it will still takes a lot of seconds to write back 
MIN_WRITEBACK_PAGES pages.

So it might be a solution, but given the fact that the current minimum 
chunk size (1024) has been in place for over ten years, and that making 
it smaller would probably have a negative impact on performance. I'm 
afraid the filesystem maintainers will not accept this change.
If we don’t modify this part but can report progress after finishing the 
chunk writeback, it should probably eliminate most of the unexpected 
warnings.
> 
>> If we want to make the hung task detector to known the progress of writeback
>> work, we need to add some code within do_writepages(): after each finish of
>> a_ops->writepages(), we need to make detector to known there's progress.
>> Something like this:
>>
>> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
>> index 3e248d1c3969..49572a83c47b 100644
>> --- a/mm/page-writeback.c
>> +++ b/mm/page-writeback.c
>> @@ -2635,6 +2635,10 @@ int do_writepages(struct address_space *mapping,
>> struct writeback_control *wbc)
>>                  else
>>                          /* deal with chardevs and other special files */
>>                          ret = 0;
>> +               /* Make hung task detector to known there's progress. */
>> +               if (force_wake)
>> +                       wake_up_all(waitq);
>> +
>>                  if (ret != -ENOMEM || wbc->sync_mode != WB_SYNC_ALL)
>>                          break;
>>
>> which has a big impact on current code - I don't want to introduce this.
> 
> You sure? It looks to me like the next level down is wb_writeback() and
> writeback_sb_inodes(), and those already have time based breaks in and
> still have access to wb_writeback_work::done, while do_writepages() no
> longer has that context.

Yeah, exactly. What I mean is report progress within the whole writeback 
work, either writeback_sb_inodes() or do_writepages() is ok.
> 
>> Yes, the behavior in this patch does have the possibility to paper cover the
>> real warnings, and what I want to argue is that the essence of this patch is
>> the same as the current touch_nmi_watchdog() and touch_softlockup_watchdog()
>> - these functions are used only in specific scenarios we known and only
>> affect a single event. And there seems no report that
>> touch_nmi/softlockup_watchdog() will paper cover the real warnings (do we?).
>>
>> Correct me if there's anything I'm missing or misunderstanding.
> 
> The thing with touch_nmi_watchdog() is that you need to keep doing it.
> The moment you stop calling touch_nmi_watchdog(), you will cause it to
> fire.
> 
> That is very much in line with the thing I proposed, and rather unlike
> your proposal that blanket kill reporting for the task, irrespective of
> how long it sits there waiting.
> 

Thanks for clarification. So how about the following solution?

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index a07b8cf73ae2..e0698fd3f9ab 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,7 @@ 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->stamp = jiffies; 
!atomic_read(&done->cnt)));
  }

  #ifdef CONFIG_CGROUP_WRITEBACK
@@ -1975,6 +1976,11 @@ static long writeback_sb_inodes(struct 
super_block *sb,
                  */
                 __writeback_single_inode(inode, &wbc);

+               /* Report progress to make hung task detector know it. */
+               if (jiffies - work->done->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;

Instead of waking up all waiting threads every half second, we only wake 
them up if the writeback work lasts for the value of 
sysctl_hung_task_timeout_secs / 2 seconds to reduce possible overhead.

Hi, Jan, Christian, how do you think about it?

Please correct me if there's anything I'm missing or misunderstanding.

Thanks,
-- 
Julian Sun <sunjunchao@bytedance.com>

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

* Re: [PATCH v2 1/2] hung_task: Introduce touch_hung_task_detector().
  2025-09-24  3:40 ` [PATCH v2 1/2] hung_task: Introduce touch_hung_task_detector() Julian Sun
@ 2025-09-25 13:33   ` Masami Hiramatsu
  0 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2025-09-25 13:33 UTC (permalink / raw)
  To: Julian Sun
  Cc: cgroups, linux-kernel, akpm, lance.yang, mhiramat, yangyicong,
	will, dianders, mingo, lihuafei1, hannes, mhocko, roman.gushchin,
	shakeel.butt, muchun.song, tj, peterz

On Wed, 24 Sep 2025 11:40:59 +0800
Julian Sun <sunjunchao@bytedance.com> wrote:

> In the kernel, long waits can trigger hung task warnings. However, some
> warnings are undesirable and unnecessary - for example, a hung task
> warning triggered when a background kworker waits for writeback
> completion during resource cleanup(like the context of
> mem_cgroup_css_free()). This kworker does not affect any user behavior
> and there is no erroneous behavior at the kernel code level, yet it
> triggers an annoying hung task warning.
> 
> To eliminate such warnings, this patch introduces
> touch_hung_task_detector() to allow some tasks ignored by hung task
> detector.
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks,

> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Suggested-by: Lance Yang <lance.yang@linux.dev>
> ---
>  include/linux/nmi.h |  2 ++
>  kernel/hung_task.c  | 13 +++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
> index cf3c6ab408aa..61fc2ad234de 100644
> --- a/include/linux/nmi.h
> +++ b/include/linux/nmi.h
> @@ -59,8 +59,10 @@ static inline void touch_all_softlockup_watchdogs(void) { }
>  
>  #ifdef CONFIG_DETECT_HUNG_TASK
>  void reset_hung_task_detector(void);
> +void touch_hung_task_detector(struct task_struct *t);
>  #else
>  static inline void reset_hung_task_detector(void) { }
> +static inline void touch_hung_task_detector(struct task_struct *t) { }
>  #endif
>  
>  /*
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index 8708a1205f82..6409d3d4bd36 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -184,6 +184,11 @@ static inline void debug_show_blocker(struct task_struct *task)
>  }
>  #endif
>  
> +void touch_hung_task_detector(struct task_struct *t)
> +{
> +	t->last_switch_count = ULONG_MAX;
> +}
> +
>  static void check_hung_task(struct task_struct *t, unsigned long timeout)
>  {
>  	unsigned long switch_count = t->nvcsw + t->nivcsw;
> @@ -203,6 +208,10 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
>  	if (unlikely(!switch_count))
>  		return;
>  
> +	/* The task doesn't want to trigger the hung task warning. */
> +	if (unlikely(t->last_switch_count == ULONG_MAX))
> +		return;
> +
>  	if (switch_count != t->last_switch_count) {
>  		t->last_switch_count = switch_count;
>  		t->last_switch_time = jiffies;
> @@ -317,6 +326,10 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
>  		    !(state & TASK_WAKEKILL) &&
>  		    !(state & TASK_NOLOAD))
>  			check_hung_task(t, timeout);
> +		else if (unlikely(t->last_switch_count == ULONG_MAX)) {
> +			t->last_switch_count = t->nvcsw + t->nivcsw;
> +			t->last_switch_time = jiffies;
> +		}
>  	}
>   unlock:
>  	rcu_read_unlock();
> -- 
> 2.39.5
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

end of thread, other threads:[~2025-09-25 13:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-24  3:40 [PATCH v2 0/2] Suppress undesirable hung task warnings Julian Sun
2025-09-24  3:40 ` [PATCH v2 1/2] hung_task: Introduce touch_hung_task_detector() Julian Sun
2025-09-25 13:33   ` Masami Hiramatsu
2025-09-24  3:41 ` [PATCH v2 2/2] memcg: Don't trigger hung task warnings when memcg is releasing resources Julian Sun
2025-09-24  6:32   ` Peter Zijlstra
2025-09-24  7:50     ` [External] " Julian Sun
2025-09-24  8:28       ` Peter Zijlstra
2025-09-24 10:36         ` Julian Sun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®