From: Colin Cross <ccross@android.com>
To: linux-kernel@vger.kernel.org
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>,
Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
"Rafael J. Wysocki" <rjw@sisk.pl>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
"J. Bruce Fields" <bfields@fieldses.org>,
"David S. Miller" <davem@davemloft.net>,
Andrew Morton <akpm@linux-foundation.org>,
Mandeep Singh Baines <msb@chromium.org>,
Paul Walmsley <paul@pwsan.com>, Colin Cross <ccross@android.com>,
Al Viro <viro@zeniv.linux.org.uk>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Oleg Nesterov <oleg@redhat.com>,
linux-nfs@vger.kernel.org, linux-pm@vger.kernel.org,
netdev@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>,
Tejun Heo <tj@kernel.org>, Ben Chan <benchan@chromium.org>
Subject: [PATCH 2/2] lockdep: check that no locks held at freeze time
Date: Fri, 3 May 2013 14:04:10 -0700 [thread overview]
Message-ID: <1367615050-3894-2-git-send-email-ccross@android.com> (raw)
In-Reply-To: <1367615050-3894-1-git-send-email-ccross@android.com>
From: Mandeep Singh Baines <msb@chromium.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.
History:
This patch was originally applied as 6aa9707099c and reverted in
dbf520a9d7d4 because NFS was freezing with locks held. It was
deemed better to keep the bad freeze point in NFS to allow laptops
to suspend consistently. The previous patch in this series converts
NFS to call _unsafe versions of the freezable helpers so that
lockdep doesn't complain about them until a more correct fix
can be applied.
[akpm@linux-foundation.org: export debug_check_no_locks_held]
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Cc: Ben Chan <benchan@chromium.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[ccross@android.com: don't warn if try_to_freeze_unsafe is called]
Signed-off-by: Colin Cross <ccross@android.com>
---
include/linux/debug_locks.h | 4 ++--
include/linux/freezer.h | 3 +++
kernel/exit.c | 2 +-
kernel/lockdep.c | 17 ++++++++---------
4 files changed, 14 insertions(+), 12 deletions(-)
As requested by Tejun, this is to prevent incorrect use when
I add new freezable helpers in a subsequent patch series.
I'm not sure what the protocol is for Signed-off-by when
reapplying a reverted patch, but I got the patch from Linus'
tree so I left all of them and added mine at the bottom.
diff --git a/include/linux/debug_locks.h b/include/linux/debug_locks.h
index 3bd46f7..a975de1 100644
--- 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)
{
@@ -67,7 +67,7 @@ debug_check_no_locks_freed(const void *from, unsigned long len)
}
static inline void
-debug_check_no_locks_held(struct task_struct *task)
+debug_check_no_locks_held(void)
{
}
#endif
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 5b31e21c..efb80dd 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -3,6 +3,7 @@
#ifndef FREEZER_H_INCLUDED
#define FREEZER_H_INCLUDED
+#include <linux/debug_locks.h>
#include <linux/sched.h>
#include <linux/wait.h>
#include <linux/atomic.h>
@@ -60,6 +61,8 @@ static inline bool try_to_freeze_unsafe(void)
static inline bool try_to_freeze(void)
{
+ if (!(current->flags & PF_NOFREEZE))
+ debug_check_no_locks_held();
return try_to_freeze_unsafe();
}
diff --git a/kernel/exit.c b/kernel/exit.c
index 60bc027..51e485c 100644
--- 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();
/*
* We can do this unlocked here. The futex code uses this flag
* just to verify whether the pi state cleanup has been done
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 8a0efac..259db20 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -4088,7 +4088,7 @@ void debug_check_no_locks_freed(const void *mem_from, unsigned long mem_len)
}
EXPORT_SYMBOL_GPL(debug_check_no_locks_freed);
-static void print_held_locks_bug(struct task_struct *curr)
+static void print_held_locks_bug(void)
{
if (!debug_locks_off())
return;
@@ -4097,22 +4097,21 @@ static void print_held_locks_bug(struct task_struct *curr)
printk("\n");
printk("=====================================\n");
- printk("[ BUG: lock held at task exit time! ]\n");
+ printk("[ BUG: %s/%d still has locks held! ]\n",
+ current->comm, task_pid_nr(current));
print_kernel_ident();
printk("-------------------------------------\n");
- printk("%s/%d is exiting with locks still held!\n",
- curr->comm, task_pid_nr(curr));
- lockdep_print_held_locks(curr);
-
+ lockdep_print_held_locks(current);
printk("\nstack backtrace:\n");
dump_stack();
}
-void debug_check_no_locks_held(struct task_struct *task)
+void debug_check_no_locks_held(void)
{
- if (unlikely(task->lockdep_depth > 0))
- print_held_locks_bug(task);
+ if (unlikely(current->lockdep_depth > 0))
+ print_held_locks_bug();
}
+EXPORT_SYMBOL_GPL(debug_check_no_locks_held);
void debug_show_all_locks(void)
{
--
1.8.2.1
next prev parent reply other threads:[~2013-05-03 21:04 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-03 21:04 [PATCH 1/2] freezer: add unsafe versions of freezable helpers Colin Cross
2013-05-03 21:04 ` Colin Cross [this message]
[not found] ` <1367615050-3894-2-git-send-email-ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
2013-05-04 13:04 ` [PATCH 2/2] lockdep: check that no locks held at freeze time Pavel Machek
2013-05-04 20:27 ` Colin Cross
2013-05-04 22:57 ` Pavel Machek
[not found] ` <20130504225715.GB24276-tWAi6jLit6GreWDznjuHag@public.gmane.org>
2013-05-04 23:49 ` Colin Cross
2013-05-05 0:05 ` Pavel Machek
[not found] ` <20130505000528.GA25454-tWAi6jLit6GreWDznjuHag@public.gmane.org>
2013-05-05 0:23 ` Colin Cross
[not found] ` <CAMbhsRQso4fW_DVL6U3zfbX3YbLsTy8-rUA-fo61Aw93bU+sQQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-05-05 1:13 ` Pavel Machek
2013-05-05 9:18 ` Ingo Molnar
[not found] ` <20130505091844.GC22239-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-05-06 8:55 ` Peter Zijlstra
2013-05-06 12:11 ` Pavel Machek
2013-05-06 14:33 ` Linus Torvalds
2013-05-06 14:42 ` Peter Zijlstra
2013-05-06 19:01 ` Tejun Heo
2013-05-06 19:30 ` Colin Cross
2013-05-06 19:33 ` Tejun Heo
2013-05-06 20:06 ` Rafael J. Wysocki
2013-05-04 13:00 ` [PATCH 1/2] freezer: add unsafe versions of freezable helpers Pavel Machek
2013-05-04 20:23 ` Colin Cross
[not found] ` <CAMbhsRS6+hjTmrihVzgu3Dtyp8XAhJJ4VKMj=28G6xH3H73=6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-05-04 22:55 ` Pavel Machek
2013-05-05 9:23 ` Ingo Molnar
[not found] ` <20130505092318.GD22239-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-05-05 22:12 ` Pavel Machek
[not found] ` <1367615050-3894-1-git-send-email-ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
2013-05-06 8:50 ` Peter Zijlstra
2013-05-06 10:58 ` Jeff Layton
2013-05-06 18:55 ` Tejun Heo
2013-05-06 10:56 ` Jeff Layton
2013-05-06 19:57 ` Colin Cross
[not found] ` <CAMbhsRQsFT2j_CuPL45J03itymcp3PNP8ckt3fAAo+hzavNrbw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-05-06 21:43 ` Jeff Layton
[not found] ` <20130506174336.447d0d75-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2013-05-06 21:54 ` Colin Cross
2013-05-06 21:58 ` Linus Torvalds
2013-05-06 22:05 ` Jeff Layton
2013-05-06 22:11 ` Colin Cross
2013-05-06 22:14 ` Tejun Heo
2013-05-06 21:59 ` Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1367615050-3894-2-git-send-email-ccross@android.com \
--to=ccross@android.com \
--cc=Trond.Myklebust@netapp.com \
--cc=akpm@linux-foundation.org \
--cc=benchan@chromium.org \
--cc=bfields@fieldses.org \
--cc=davem@davemloft.net \
--cc=ebiederm@xmission.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=msb@chromium.org \
--cc=netdev@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=paul@pwsan.com \
--cc=pavel@ucw.cz \
--cc=peterz@infradead.org \
--cc=rjw@sisk.pl \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).