* [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
* 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
* [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
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®