diff for duplicates of <87o9y2a5ji.fsf@yhuang-dev.intel.com> diff --git a/a/1.txt b/N1/1.txt index aaebccd..5d8aa8b 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -15,3 +15,127 @@ Best Regards, Huang, Ying -------------------------------------------------------------> +>From 2b9e2f78a6e389442f308c4f9e8d5ac40fe6aa2f Mon Sep 17 00:00:00 2001 +From: Huang Ying <ying.huang@intel.com> +Date: Thu, 16 Feb 2017 16:38:17 +0800 +Subject: [PATCH] mm, swap: Annotate nested locking for cluster lock + +There is a nested locking in cluster_list_add_tail() for cluster lock, +which caused lockdep to complain as below. The nested locking is safe +because both cluster locks are only acquired when we held the +swap_info_struct->lock. Annotated the nested locking via +spin_lock_nested() to fix the complain of lockdep. + +============================================= +[ INFO: possible recursive locking detected ] +4.10.0-rc8-next-20170214-zram #24 Not tainted +--------------------------------------------- +as/6557 is trying to acquire lock: + (&(&((cluster_info + ci)->lock))->rlock){+.+.-.}, at: [<ffffffff811ddd03>] cluster_list_add_tail.part.31+0x33/0x70 + +but task is already holding lock: + (&(&((cluster_info + ci)->lock))->rlock){+.+.-.}, at: [<ffffffff811df2bb>] swapcache_free_entries+0x9b/0x330 + +other info that might help us debug this: + Possible unsafe locking scenario: + + CPU0 + ---- + lock(&(&((cluster_info + ci)->lock))->rlock); + lock(&(&((cluster_info + ci)->lock))->rlock); + + *** DEADLOCK *** + + May be due to missing lock nesting notation + +3 locks held by as/6557: + #0: (&(&cache->free_lock)->rlock){......}, at: [<ffffffff811c206b>] free_swap_slot+0x8b/0x110 + #1: (&(&p->lock)->rlock){+.+.-.}, at: [<ffffffff811df295>] swapcache_free_entries+0x75/0x330 + #2: (&(&((cluster_info + ci)->lock))->rlock){+.+.-.}, at: [<ffffffff811df2bb>] swapcache_free_entries+0x9b/0x330 + +stack backtrace: +CPU: 3 PID: 6557 Comm: as Not tainted 4.10.0-rc8-next-20170214-zram #24 +Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 +Call Trace: + dump_stack+0x85/0xc2 + __lock_acquire+0x15ea/0x1640 + lock_acquire+0x100/0x1f0 + ? cluster_list_add_tail.part.31+0x33/0x70 + _raw_spin_lock+0x38/0x50 + ? cluster_list_add_tail.part.31+0x33/0x70 + cluster_list_add_tail.part.31+0x33/0x70 + swapcache_free_entries+0x2f9/0x330 + free_swap_slot+0xf8/0x110 + swapcache_free+0x36/0x40 + delete_from_swap_cache+0x5f/0xa0 + try_to_free_swap+0x6e/0xa0 + free_pages_and_swap_cache+0x7d/0xb0 + tlb_flush_mmu_free+0x36/0x60 + tlb_finish_mmu+0x1c/0x50 + exit_mmap+0xc7/0x150 + mmput+0x51/0x110 + do_exit+0x2b2/0xc30 + ? trace_hardirqs_on_caller+0x129/0x1b0 + do_group_exit+0x50/0xd0 + SyS_exit_group+0x14/0x20 + entry_SYSCALL_64_fastpath+0x23/0xc6 +RIP: 0033:0x2b9a2dbdf309 +RSP: 002b:00007ffe71887528 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 +RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00002b9a2dbdf309 +RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 +RBP: 00002b9a2ded8858 R08: 000000000000003c R09: 00000000000000e7 +R10: ffffffffffffff60 R11: 0000000000000246 R12: 00002b9a2ded8858 +R13: 00002b9a2dedde80 R14: 000000000255f770 R15: 0000000000000001 + +Reported-by: Minchan Kim <minchan@kernel.org> +Signed-off-by: "Huang, Ying" <ying.huang@intel.com> +--- + include/linux/swap.h | 6 ++++++ + mm/swapfile.c | 8 +++++++- + 2 files changed, 13 insertions(+), 1 deletion(-) + +diff --git a/include/linux/swap.h b/include/linux/swap.h +index 4d12b381821f..ef044ea8fe79 100644 +--- a/include/linux/swap.h ++++ b/include/linux/swap.h +@@ -166,6 +166,12 @@ enum { + #define COUNT_CONTINUED 0x80 /* See swap_map continuation for full count */ + #define SWAP_MAP_SHMEM 0xbf /* Owned by shmem/tmpfs, in first swap_map */ + ++enum swap_cluster_lock_class ++{ ++ SWAP_CLUSTER_LOCK_NORMAL, /* implicitly used by plain spin_lock() APIs. */ ++ SWAP_CLUSTER_LOCK_NESTED, ++}; ++ + /* + * We use this to track usage of a cluster. A cluster is a block of swap disk + * space with SWAPFILE_CLUSTER pages long and naturally aligns in disk. All +diff --git a/mm/swapfile.c b/mm/swapfile.c +index 5ac2cb40dbd3..0a52e9b2f843 100644 +--- a/mm/swapfile.c ++++ b/mm/swapfile.c +@@ -263,6 +263,12 @@ static inline void __lock_cluster(struct swap_cluster_info *ci) + spin_lock(&ci->lock); + } + ++static inline void __lock_cluster_nested(struct swap_cluster_info *ci, ++ unsigned subclass) ++{ ++ spin_lock_nested(&ci->lock, subclass); ++} ++ + static inline struct swap_cluster_info *lock_cluster(struct swap_info_struct *si, + unsigned long offset) + { +@@ -336,7 +342,7 @@ static void cluster_list_add_tail(struct swap_cluster_list *list, + * only acquired when we held swap_info_struct->lock + */ + ci_tail = ci + tail; +- __lock_cluster(ci_tail); ++ __lock_cluster_nested(ci_tail, SWAP_CLUSTER_LOCK_NESTED); + cluster_set_next(ci_tail, idx); + unlock_cluster(ci_tail); + cluster_set_next_flag(&list->tail, idx, 0); +-- +2.11.0 diff --git a/a/content_digest b/N1/content_digest index 4306219..1f5c2ce 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -3,13 +3,13 @@ "Subject\0Re: swap_cluster_info lockdep splat\0" "Date\0Thu, 16 Feb 2017 16:44:33 +0800\0" "To\0Minchan Kim <minchan@kernel.org>\0" - "Cc\0Huang" + "Cc\0Huang\\" Ying <ying.huang@intel.com> Andrew Morton <akpm@linux-foundation.org> Tim Chen <tim.c.chen@linux.intel.com> Hugh Dickins <hughd@google.com> - linux-kernel@vger.kernel.org - " linux-mm@kvack.org\0" + <linux-kernel@vger.kernel.org> + " <linux-mm@kvack.org>\0" "\00:1\0" "b\0" "Hi, Minchan,\n" @@ -28,6 +28,130 @@ "Best Regards,\n" "Huang, Ying\n" "\n" - -------------------------------------------------------------> + "------------------------------------------------------------->\n" + ">From 2b9e2f78a6e389442f308c4f9e8d5ac40fe6aa2f Mon Sep 17 00:00:00 2001\n" + "From: Huang Ying <ying.huang@intel.com>\n" + "Date: Thu, 16 Feb 2017 16:38:17 +0800\n" + "Subject: [PATCH] mm, swap: Annotate nested locking for cluster lock\n" + "\n" + "There is a nested locking in cluster_list_add_tail() for cluster lock,\n" + "which caused lockdep to complain as below. The nested locking is safe\n" + "because both cluster locks are only acquired when we held the\n" + "swap_info_struct->lock. Annotated the nested locking via\n" + "spin_lock_nested() to fix the complain of lockdep.\n" + "\n" + "=============================================\n" + "[ INFO: possible recursive locking detected ]\n" + "4.10.0-rc8-next-20170214-zram #24 Not tainted\n" + "---------------------------------------------\n" + "as/6557 is trying to acquire lock:\n" + " (&(&((cluster_info + ci)->lock))->rlock){+.+.-.}, at: [<ffffffff811ddd03>] cluster_list_add_tail.part.31+0x33/0x70\n" + "\n" + "but task is already holding lock:\n" + " (&(&((cluster_info + ci)->lock))->rlock){+.+.-.}, at: [<ffffffff811df2bb>] swapcache_free_entries+0x9b/0x330\n" + "\n" + "other info that might help us debug this:\n" + " Possible unsafe locking scenario:\n" + "\n" + " CPU0\n" + " ----\n" + " lock(&(&((cluster_info + ci)->lock))->rlock);\n" + " lock(&(&((cluster_info + ci)->lock))->rlock);\n" + "\n" + " *** DEADLOCK ***\n" + "\n" + " May be due to missing lock nesting notation\n" + "\n" + "3 locks held by as/6557:\n" + " #0: (&(&cache->free_lock)->rlock){......}, at: [<ffffffff811c206b>] free_swap_slot+0x8b/0x110\n" + " #1: (&(&p->lock)->rlock){+.+.-.}, at: [<ffffffff811df295>] swapcache_free_entries+0x75/0x330\n" + " #2: (&(&((cluster_info + ci)->lock))->rlock){+.+.-.}, at: [<ffffffff811df2bb>] swapcache_free_entries+0x9b/0x330\n" + "\n" + "stack backtrace:\n" + "CPU: 3 PID: 6557 Comm: as Not tainted 4.10.0-rc8-next-20170214-zram #24\n" + "Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014\n" + "Call Trace:\n" + " dump_stack+0x85/0xc2\n" + " __lock_acquire+0x15ea/0x1640\n" + " lock_acquire+0x100/0x1f0\n" + " ? cluster_list_add_tail.part.31+0x33/0x70\n" + " _raw_spin_lock+0x38/0x50\n" + " ? cluster_list_add_tail.part.31+0x33/0x70\n" + " cluster_list_add_tail.part.31+0x33/0x70\n" + " swapcache_free_entries+0x2f9/0x330\n" + " free_swap_slot+0xf8/0x110\n" + " swapcache_free+0x36/0x40\n" + " delete_from_swap_cache+0x5f/0xa0\n" + " try_to_free_swap+0x6e/0xa0\n" + " free_pages_and_swap_cache+0x7d/0xb0\n" + " tlb_flush_mmu_free+0x36/0x60\n" + " tlb_finish_mmu+0x1c/0x50\n" + " exit_mmap+0xc7/0x150\n" + " mmput+0x51/0x110\n" + " do_exit+0x2b2/0xc30\n" + " ? trace_hardirqs_on_caller+0x129/0x1b0\n" + " do_group_exit+0x50/0xd0\n" + " SyS_exit_group+0x14/0x20\n" + " entry_SYSCALL_64_fastpath+0x23/0xc6\n" + "RIP: 0033:0x2b9a2dbdf309\n" + "RSP: 002b:00007ffe71887528 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7\n" + "RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00002b9a2dbdf309\n" + "RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000\n" + "RBP: 00002b9a2ded8858 R08: 000000000000003c R09: 00000000000000e7\n" + "R10: ffffffffffffff60 R11: 0000000000000246 R12: 00002b9a2ded8858\n" + "R13: 00002b9a2dedde80 R14: 000000000255f770 R15: 0000000000000001\n" + "\n" + "Reported-by: Minchan Kim <minchan@kernel.org>\n" + "Signed-off-by: \"Huang, Ying\" <ying.huang@intel.com>\n" + "---\n" + " include/linux/swap.h | 6 ++++++\n" + " mm/swapfile.c | 8 +++++++-\n" + " 2 files changed, 13 insertions(+), 1 deletion(-)\n" + "\n" + "diff --git a/include/linux/swap.h b/include/linux/swap.h\n" + "index 4d12b381821f..ef044ea8fe79 100644\n" + "--- a/include/linux/swap.h\n" + "+++ b/include/linux/swap.h\n" + "@@ -166,6 +166,12 @@ enum {\n" + " #define COUNT_CONTINUED\t0x80\t/* See swap_map continuation for full count */\n" + " #define SWAP_MAP_SHMEM\t0xbf\t/* Owned by shmem/tmpfs, in first swap_map */\n" + " \n" + "+enum swap_cluster_lock_class\n" + "+{\n" + "+\tSWAP_CLUSTER_LOCK_NORMAL, /* implicitly used by plain spin_lock() APIs. */\n" + "+\tSWAP_CLUSTER_LOCK_NESTED,\n" + "+};\n" + "+\n" + " /*\n" + " * We use this to track usage of a cluster. A cluster is a block of swap disk\n" + " * space with SWAPFILE_CLUSTER pages long and naturally aligns in disk. All\n" + "diff --git a/mm/swapfile.c b/mm/swapfile.c\n" + "index 5ac2cb40dbd3..0a52e9b2f843 100644\n" + "--- a/mm/swapfile.c\n" + "+++ b/mm/swapfile.c\n" + "@@ -263,6 +263,12 @@ static inline void __lock_cluster(struct swap_cluster_info *ci)\n" + " \tspin_lock(&ci->lock);\n" + " }\n" + " \n" + "+static inline void __lock_cluster_nested(struct swap_cluster_info *ci,\n" + "+\t\t\t\t\t unsigned subclass)\n" + "+{\n" + "+\tspin_lock_nested(&ci->lock, subclass);\n" + "+}\n" + "+\n" + " static inline struct swap_cluster_info *lock_cluster(struct swap_info_struct *si,\n" + " \t\t\t\t\t\t unsigned long offset)\n" + " {\n" + "@@ -336,7 +342,7 @@ static void cluster_list_add_tail(struct swap_cluster_list *list,\n" + " \t\t * only acquired when we held swap_info_struct->lock\n" + " \t\t */\n" + " \t\tci_tail = ci + tail;\n" + "-\t\t__lock_cluster(ci_tail);\n" + "+\t\t__lock_cluster_nested(ci_tail, SWAP_CLUSTER_LOCK_NESTED);\n" + " \t\tcluster_set_next(ci_tail, idx);\n" + " \t\tunlock_cluster(ci_tail);\n" + " \t\tcluster_set_next_flag(&list->tail, idx, 0);\n" + "-- \n" + 2.11.0 -34b068f8335fb3b2c81658c7ae84e43e63750db5f408b8b88ee471d02b93c0bf +f032ae47d0944de9b621d1cf5e04954a2218cec9f30d494d382cd71b34c8e11e
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.