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  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]
Date:	Sat, 4 May 2013 16:49:56 -0700
From:	Colin Cross <ccross@...roid.com>
To:	Pavel Machek <pavel@....cz>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	Trond Myklebust <Trond.Myklebust@...app.com>,
	Len Brown <len.brown@...el.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	"J. Bruce Fields" <bfields@...ldses.org>,
	"David S. Miller" <davem@...emloft.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mandeep Singh Baines <msb@...omium.org>,
	Paul Walmsley <paul@...an.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Oleg Nesterov <oleg@...hat.com>, linux-nfs@...r.kernel.org,
	Linux PM list <linux-pm@...r.kernel.org>,
	netdev <netdev@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>, Ben Chan <benchan@...omium.org>
Subject: Re: [PATCH 2/2] lockdep: check that no locks held at freeze time

On Sat, May 4, 2013 at 3:57 PM, Pavel Machek <pavel@....cz> wrote:
> On Sat 2013-05-04 13:27:23, Colin Cross wrote:
>> On Sat, May 4, 2013 at 6:04 AM, Pavel Machek <pavel@....cz> wrote:
>> > On Fri 2013-05-03 14:04:10, Colin Cross wrote:
>> >> From: Mandeep Singh Baines <msb@...omium.org>
>> >>
>> >> 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.
>
> Well, it was also reverted once :-).

It was reverted because of problems in NFS, not because of any problem
with this patch.

>> >> --- 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;
>
> Aha, I understand it now.
>
> Accessing current is slower than local variable. So your "new" code
> will work but will be slower. Please revert this part.

Using current instead of passing in tsk was done at Andrew Morton's
suggestion, and makes no difference from the freezer's perspective
since it would have to use current to get the task to pass in, so I'm
going to leave it as is.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists