linux-kernel - Re: [PATCH 2/2 v3] lib: debugobjects: touch watchdog to avoid softlockup when !CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e2d8c024-169c-0a08-41f4-5a03034fe05b@alibaba-inc.com>
Date:   Thu, 18 Jan 2018 09:41:45 +0800
From:   "Yang Shi" <yang.s@...baba-inc.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     longman@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2 v3] lib: debugobjects: touch watchdog to avoid
 softlockup when !CONFIG_PREEMPT



On 1/17/18 4:21 AM, Thomas Gleixner wrote:
> On Thu, 4 Jan 2018, Yang Shi wrote:
> 
>> There are nested loops on debug objects free path, sometimes it may take
>> over hundred thousands of loops, then cause soft lockup with !CONFIG_PREEMPT
>> occasionally, like below:
> 
> Please trim back traces. The whole module info and whatever is completely
> irrelevant.

Yes, sure.

> 
>> @@ -768,6 +771,10 @@ static void __debug_check_no_obj_freed(const void *address, unsigned long size)
>>   			debug_objects_maxchain = cnt;
>>   
>>   		max_loops += cnt;
>> +
>> +		if (max_loops > 10000 && ((max_loops % 10000) == 0)
>> +		    && suppress_lockup != 0)
>> +			touch_softlockup_watchdog();
> 
> This is voodoo programming.
> 
> There are two things which can be done here:
> 
>   1) The collected objects can be put on a global free list and work
>      scheduled to free them piecewise.

I don't get your point here. objects free has already been done in a 
work. free_object() -> schedule_work()

Do you mean free all of them out of the for loop in a batch? Then don't 
call free_object() in the for loop?

> 
>   2) We can do a cond_resched() if not in atomic context and interrupts are
>      enabled.

I did try this before I went with touching softlockup watchdog approach. 
The problem is in_atomic() can't tell if it is in atomic context on 
non-preempt kernel. For preempt kernel, it is easy.

Thanks,
Yang

> 
> I rather prefer to make that stuff robust than having crystal ball
> constants and magic debugfs knobs.
> 
> Thanks,
> 
> 	tglx
>