On 6/15/26 16:14, Pavel Tikhomirov wrote:
> Reviewed-by: Pavel Tikhomirov <[email protected]>
> 
> On 6/4/26 11:44, Konstantin Khorenko wrote:
>> warn_freeze_timeout() holds guard(rcu)() for its whole body (required by
>> css_for_each_descendant_post()) and, inside that, both
>> warn_freeze_timeout_task() and the "no unfreezable process" tail do
>> kmalloc(PATH_MAX / stack-trace buffer, GFP_KERNEL). GFP_KERNEL may sleep,
>> which is illegal inside an RCU read-side critical section ("sleeping
>> function called from invalid context", caught by
>> CONFIG_DEBUG_ATOMIC_SLEEP). The path is reachable from
>> cgroup.events show -> check_freeze_timeout() -> warn_freeze_timeout() when
>> a freeze exceeds sysctl_freeze_timeout and passes the ratelimit.
>>
>> The non-allocating work here (cgroup_path(), stack_trace_save_tsk()) is
>> atomic-safe; only the allocations sleep. Use GFP_ATOMIC for the three
>> buffers - this is a rare, ratelimited diagnostic path with small (<=PATH_MAX)
>> allocations, so the stricter allocation context is acceptable.
> 
> Looks good. Alternatively, if we would need it in future, we can take a 
> reference
> on the task and release the rcu lock before calling 
> warn_freeze_timeout_task().

Added the comment to the commit message.

>>
>> Fixes: b55c88db815e ("cgroup-v2/freezer: Print information about unfreezable 
>> process")
>> https://virtuozzo.atlassian.net/browse/VSTOR-132310
>> Signed-off-by: Konstantin Khorenko <[email protected]>
>> ---
>>  kernel/cgroup/cgroup.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
>> index e37df4ccb51e..14e93647e999 100644
>> --- a/kernel/cgroup/cgroup.c
>> +++ b/kernel/cgroup/cgroup.c
>> @@ -4250,7 +4250,13 @@ void warn_freeze_timeout_task(struct cgroup *cgrp, 
>> int timeout,
>>      unsigned long nr_entries, i;
>>      pid_t tgid;
>>  
>> -    buf = kmalloc(PATH_MAX, GFP_KERNEL);
>> +    /*
>> +     * Called from warn_freeze_timeout() under rcu_read_lock() (the
>> +     * css_for_each_descendant_post() iteration), so allocations here must
>> +     * not sleep.  This is a rare, ratelimited diagnostic path with small
>> +     * buffers, so GFP_ATOMIC is fine.
>> +     */
>> +    buf = kmalloc(PATH_MAX, GFP_ATOMIC);
>>      if (!buf)
>>              return;
>>  
>> @@ -4263,7 +4269,7 @@ void warn_freeze_timeout_task(struct cgroup *cgrp, int 
>> timeout,
>>             buf, timeout/HZ, tgid, task->comm);
>>  
>>      entries = kmalloc(MAX_STACK_TRACE_DEPTH * sizeof(*entries),
>> -                      GFP_KERNEL);
>> +                      GFP_ATOMIC);
>>      if (!entries)
>>              return;
>>      nr_entries = stack_trace_save_tsk(task, entries,
>> @@ -4297,7 +4303,8 @@ static void warn_freeze_timeout(struct cgroup *cgrp, 
>> int timeout)
>>              css_task_iter_end(&it);
>>      }
>>  
>> -    buf = kmalloc(PATH_MAX, GFP_KERNEL);
>> +    /* still under rcu_read_lock() from guard(rcu)() above */
>> +    buf = kmalloc(PATH_MAX, GFP_ATOMIC);
>>      if (!buf)
>>              return;
>>  
> 

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to