Re: [PATCH v2] debugobjects: Move printk out of db lock critical sections

[Waiman Long]

On 12/17/2018 01:17 PM, Ingo Molnar wrote:
> * Waiman Long <longman@xxxxxxxxxx> wrote:
>
>> The db->lock is a raw spinlock and so the lock hold time is supposed to
>> be short. This will not be the case when printk() is being involved in
>> some of the critical sections.
>>
>> In order to avoid the long hold time, in case some messages need to be
>> printed, all the debug_object_is_on_stack() and debug_print_object()
>> calls are now moved out of those critical sections in the following
>> functions.
>>
>>  - __debug_object_init()
>>  - debug_object_activate()
>>  - debug_object_deactivate()
>>  - debug_object_destroy()
>>  - debug_object_free()
>>  - debug_object_active_state()
>>  - __debug_check_no_obj_freed()
>>  - check_results()
>>
>> Holding the db->lock while calling printk() may lead to deadlock if
>> printk() somehow requires the allocation/freeing of debug object that
>> happens to be in the same hash bucket or a circular lock dependency
>> warning from lockdep as reported in
>>
>>   https://lkml.kernel.org/r/20181211091154.GL23332@shao2-debian
> This makes me sad - whatever happened to the principle of keeping printk 
> simple?
>
> We should rename printk() to syslog() or so, and rename early_printk() to 
> printk(), and be done with this.
>
> Thanks,
>
> 	Ingo

The circular lock dependency actually happened because of the serial
console code that was called by printk() to display the message. We have
multiple console drivers that may be called depending on the hardware.
It can be hard to make sure that none of them will allocate or free
objects during the call.

Cheers,
Longman