From patchwork Fri May 3 21:04:10 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Cross X-Patchwork-Id: 2519441 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id E57623FCA5 for ; Fri, 3 May 2013 21:04:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1763535Ab3ECVEU (ORCPT ); Fri, 3 May 2013 17:04:20 -0400 Received: from mail-yh0-f74.google.com ([209.85.213.74]:49078 "EHLO mail-yh0-f74.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761851Ab3ECVES (ORCPT ); Fri, 3 May 2013 17:04:18 -0400 Received: by mail-yh0-f74.google.com with SMTP id t59so217735yho.3 for ; Fri, 03 May 2013 14:04:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:x-gm-message-state; bh=MFUHGrszIAm4+toiCigctkI8yfr/4mb06gJD4ArdHQc=; b=op28eX6UFjT/KrcBoj1gBoNC3dHCpRV99ve+UfACCuiqqJ6Qo3LQXUyymjke57O/Cp eOKi/oekNntW69KaPRzWX4zczhCs62AFSrZt3fdEPnQnfmb5rCUWaPcuC47WE6gv1hfZ UXgJDnuusZDQQfnSidirL7cGcia0LlBAqZRB5f7lX0tm3qLw4oE0pcVl+0JFtrUP38vc Db5sKk1Dv2ZIWZ5068TwV2u6ROslQWqlrQzg6CuUYBrh+D9nnYHGKa5KtJebP0hJLeyX 04n41hsWPVW+wtIOCt/VjlLNEoKw2LLaT5zmL60Nj+LPkAI4CcItn8859bSFBDiCUeMd yvXQ== X-Received: by 10.236.161.234 with SMTP id w70mr9681100yhk.22.1367615057975; Fri, 03 May 2013 14:04:17 -0700 (PDT) Received: from corp2gmr1-2.hot.corp.google.com (corp2gmr1-2.hot.corp.google.com [172.24.189.93]) by gmr-mx.google.com with ESMTPS id u42si6279yhi.0.2013.05.03.14.04.17 for (version=TLSv1.1 cipher=AES128-SHA bits=128/128); Fri, 03 May 2013 14:04:17 -0700 (PDT) Received: from walnut.mtv.corp.google.com (walnut.mtv.corp.google.com [172.18.105.48]) by corp2gmr1-2.hot.corp.google.com (Postfix) with ESMTP id AFCCA5A4082; Fri, 3 May 2013 14:04:17 -0700 (PDT) Received: by walnut.mtv.corp.google.com (Postfix, from userid 99897) id 62423160950; Fri, 3 May 2013 14:04:17 -0700 (PDT) From: Colin Cross To: linux-kernel@vger.kernel.org Cc: Trond Myklebust , Len Brown , Pavel Machek , "Rafael J. Wysocki" , Peter Zijlstra , Ingo Molnar , "J. Bruce Fields" , "David S. Miller" , Andrew Morton , Mandeep Singh Baines , Paul Walmsley , Colin Cross , Al Viro , "Eric W. Biederman" , Oleg Nesterov , linux-nfs@vger.kernel.org, linux-pm@vger.kernel.org, netdev@vger.kernel.org, Linus Torvalds , Tejun Heo , Ben Chan Subject: [PATCH 2/2] lockdep: check that no locks held at freeze time Date: Fri, 3 May 2013 14:04:10 -0700 Message-Id: <1367615050-3894-2-git-send-email-ccross@android.com> X-Mailer: git-send-email 1.8.2.1 In-Reply-To: <1367615050-3894-1-git-send-email-ccross@android.com> References: <1367615050-3894-1-git-send-email-ccross@android.com> X-Gm-Message-State: ALoCoQkSfYkyHaON/WkSvr1mivG/xYQ7QzRXWzkXdQvKpVc5ceWQ0gF260DjyrSkwfvphdnfijegIqRUOxQ4QrCE1PTdV7DL00TvtOkkgaMdywY7pMbrHwr2avPbobMq1XR67I7B/+IIG68XUMaLsRd0Zx0405Ewt2FTPAVZ214KBujKJKwosl5FA+/NwHyGl9lxS46tGH3P Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: Mandeep Singh Baines 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 Cc: Ben Chan Cc: Oleg Nesterov Cc: Tejun Heo Cc: Rafael J. Wysocki Cc: Ingo Molnar Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds [ccross@android.com: don't warn if try_to_freeze_unsafe is called] Signed-off-by: Colin Cross --- 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 #include #include #include @@ -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) {