On 3/17/26 15:47, Aleksei Oladko wrote:
>
> On 3/17/26 3:08 PM, Vasileios Almpanis wrote:
>> lockdep reports an inconsistent lock state involving css_set_lock,
>> triggered via cgroup_events_show() -> check_freeze_timeout():
>>
>> WARNING: inconsistent lock state
>> inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
>>
>> Possible unsafe locking scenario:
>>
>> CPU0
>> ----
>> lock(css_set_lock);
>> <Interrupt>
>> lock(css_set_lock);
>>
>> *** DEADLOCK ***
>>
>> The lock is currently taken with spin_lock() in check_freeze_timeout(),
>> which does not disable interrupts. However, css_set_lock is also
>> acquired from IRQ/softirq context in RCU callbacks during task
>> teardown, making it unsafe to take the lock with interrupts enabled.
>>
>> This can lead to a deadlock if an interrupt fires while holding
>> css_set_lock and attempts to re-acquire it.
>>
>> Fix this by switching to spin_lock_irq() semantics via
>> scoped_guard(spinlock_irq, ...), ensuring interrupts are disabled
>> while holding css_set_lock in this path.
>>
>> This matches the locking requirements of css_set_lock.
>>
>> Fixes: 3cc887c3d0faa ("cgroup-v2/freezer: Print information about
>> unfreezable process")
>> https://virtuozzo.atlassian.net/browse/VSTOR-127046
>>
>> Signed-off-by: Vasileios Almpanis <[email protected]>
>>
>> Feature: cgroup/freeze: enhance logging
>> ---
>> kernel/cgroup/cgroup.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
>> index 040e7e36c238..376c9bbd1c41 100644
>> --- a/kernel/cgroup/cgroup.c
>> +++ b/kernel/cgroup/cgroup.c
>> @@ -4296,7 +4296,7 @@ static void check_freeze_timeout(struct cgroup *cgrp)
>> u64 freeze_jiffies;
>> int timeout;
>> - scoped_guard(spinlock, &css_set_lock) {
>> + scoped_guard(spinlock_irq, &css_set_lock) {
> Since the lock is only shared between process context and softirqs,
> spinlock_bh should be sufficient here. There is no need to disable irq.
Good catch, really the lockdep stack is from rcu handling and is from softirq.
(And Documentation/RCU/checklist.rst 11 confirms that in this case _bh is
enough.
But I think spin_lock_irq(&css_set_lock) would not be used all over the kernel
if it was only softirqs we should care about with this lock. Also even it's
only softirqs we should follow mainstream usage pattern in any way and they use
_irq version.
>> if (!test_bit(CGRP_FREEZE, &cgrp->flags) ||
>> test_bit(CGRP_FROZEN, &cgrp->flags))
>> return;
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
