Re: [PATCH 0/3] Suppress undesirable hung task warnings.

[Lance Yang <lance.yang@linux.dev>]

On 2025/9/22 20:40, Julian Sun wrote:
> On 9/22/25 7:38 PM, Lance Yang wrote:
> 
> Hi, Lance
> 
> Thanks for your review and comments.
> 
>> Hi Julian
>>
>> Thanks for the patch series!
>>
>> On 2025/9/22 17:41, Julian Sun wrote:
>>> As suggested by Andrew Morton in [1], we need a general mechanism
>>> that allows the hung task detector to ignore unnecessary hung
>>
>> Yep, I understand the goal is to suppress what can be a benign hung task
>> warning during memcg teardown.
>>
>>> tasks. This patch set implements this functionality.
>>>
>>> Patch 1 introduces a PF_DONT_HUNG flag. The hung task detector will
>>> ignores all tasks that have the PF_DONT_HUNG flag set.
>>
>> However, I'm concerned that the PF_DONT_HUNG flag is a bit too powerful
>> and might mask real, underlying hangs.
> 
> The flag takes effect only when wait_event_no_hung() or 
> wb_wait_for_completion_no_hung() is called, and its effect is limited to 
> a single wait event, without affecting subsequent wait events. So AFAICS 
> it will not mask real hang warnings.>

Emm... the risk of future misuse is what worries me. I would rather have
call sites actively "pet the watchdog" by periodically calling a helper
like touch_hung_task_detector(), instead of passively ignoring the detector.

>>>
>>> Patch 2 introduces wait_event_no_hung() and 
>>> wb_wait_for_completion_no_hung(),
>>> which enable the hung task detector to ignore hung tasks caused by these
>>> wait events.
>>
>> Instead of making the detector ignore the task, what if we just change
>> the waiting mechanism? Looking at wb_wait_for_completion(), we could
>> introduce a new helper that internally uses wait_event_timeout() in a
>> loop.
>>
>> Something simple like this:
>>
>> void wb_wait_for_completion_no_hung(struct wb_completion *done)
>> {
>>          atomic_dec(&done->cnt);
>>          while (atomic_read(&done->cnt))
>>                  wait_event_timeout(*done->waitq, !atomic_read(&done- 
>>  >cnt), timeout);
>> }
>>
>> The periodic wake-ups from wait_event_timeout() would naturally prevent
>> the detector from complaining about slow but eventually completing 
>> writeback.
> 
> Yeah, this could definitely eliminate the hung task warning complained 
> here.
> However what I aim to provide is a general mechanism for waiting on 
> events. Of course, we could use code similar to the following, but this 
> would introduce additional overhead from waking tasks and multiple 
> operations on wq_head—something I don't want to introduce.

Yeah, I agree there's some overhead with a polling approach, but
mem_cgroup_css_free() should be an infrequent operation. So, I think it's
an acceptable trade-off :)

> 
> +#define wait_event_no_hung(wq_head, condition) \
> +do {                   \
> +       while (!(condition))    \
> +               wait_event_timeout(wq_head, condition, timeout); \
> +}
> 
> But I can try this approach or do not introcude wait_event_no_hung() if 
> you want.>

Well, let's see what other folks think ;)

Cheers,
Lance

>>>
>>> Patch 3 uses wb_wait_for_completion_no_hung() in the final phase of 
>>> memcg
>>> teardown to eliminate the hung task warning.
>>>
>>> Julian Sun (3):
>>>    sched: Introduce a new flag PF_DONT_HUNG.
>>>    writeback: Introduce wb_wait_for_completion_no_hung().
>>>    memcg: Don't trigger hung task when memcg is releasing.
>>>
>>>   fs/fs-writeback.c           | 15 +++++++++++++++
>>>   include/linux/backing-dev.h |  1 +
>>>   include/linux/sched.h       | 12 +++++++++++-
>>>   include/linux/wait.h        | 15 +++++++++++++++
>>>   kernel/hung_task.c          |  6 ++++++
>>>   mm/memcontrol.c             |  2 +-
>>>   6 files changed, 49 insertions(+), 2 deletions(-)
>>>
>>
> 
> Thanks,