Re: [PATCH 2/2] lockdep: check that no locks held at freeze time

From: Colin Cross
Date: Sat May 04 2013 - 16:27:31 EST


On Sat, May 4, 2013 at 6:04 AM, Pavel Machek <pavel@xxxxxx> wrote:
> On Fri 2013-05-03 14:04:10, Colin Cross wrote:
>> From: Mandeep Singh Baines <msb@xxxxxxxxxxxx>
>>
>> We shouldn't try_to_freeze if locks are held. Holding a lock can cause a
>> deadlock if the lock is later acquired in the suspend or hibernate path
>> (e.g. by dpm). Holding a lock can also cause a deadlock in the case of
>> cgroup_freezer if a lock is held inside a frozen cgroup that is later
>> acquired by a process outside that group.
>
> Ok, but this does not explain why
>
>> --- a/include/linux/debug_locks.h
>> +++ b/include/linux/debug_locks.h
>> @@ -51,7 +51,7 @@ struct task_struct;
>> extern void debug_show_all_locks(void);
>> extern void debug_show_held_locks(struct task_struct *task);
>> extern void debug_check_no_locks_freed(const void *from, unsigned long len);
>> -extern void debug_check_no_locks_held(struct task_struct *task);
>> +extern void debug_check_no_locks_held(void);
>> #else
>> static inline void debug_show_all_locks(void)
>> {
>
> Removing task_struct argument from those functions is good idea?

This is an existing patch that was merged in 3.9 and then reverted
again, so it has already been reviewed and accepted once.

>> --- a/kernel/exit.c
>> +++ b/kernel/exit.c
>> @@ -835,7 +835,7 @@ void do_exit(long code)
>> /*
>> * Make sure we are holding no locks:
>> */
>> - debug_check_no_locks_held(tsk);
>> + debug_check_no_locks_held();
>
> Is task guaranteed == current?

Yes, the first line of do_exit is:
struct task_struct *tsk = current;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/