diff mbox series

mm/hugetlb: fix possible recursive locking detected warning

Message ID 20240711071001.3475337-1-linmiaohe@huawei.com (mailing list archive)
State New
Headers show
Series mm/hugetlb: fix possible recursive locking detected warning | expand

Commit Message

Miaohe Lin July 11, 2024, 7:10 a.m. UTC 
When tries to demote 1G hugetlb folios, a lockdep warning is observed:

============================================
WARNING: possible recursive locking detected
6.10.0-rc6-00452-ga4d0275fa660-dirty #79 Not tainted
--------------------------------------------
bash/710 is trying to acquire lock:
ffffffff8f0a7850 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0x244/0x460

but task is already holding lock:
ffffffff8f0a6f48 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0xae/0x460

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(&h->resize_lock);
  lock(&h->resize_lock);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

4 locks held by bash/710:
 #0: ffff8f118439c3f0 (sb_writers#5){.+.+}-{0:0}, at: ksys_write+0x64/0xe0
 #1: ffff8f11893b9e88 (&of->mutex#2){+.+.}-{3:3}, at: kernfs_fop_write_iter+0xf8/0x1d0
 #2: ffff8f1183dc4428 (kn->active#98){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x100/0x1d0
 #3: ffffffff8f0a6f48 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0xae/0x460

stack backtrace:
CPU: 3 PID: 710 Comm: bash Not tainted 6.10.0-rc6-00452-ga4d0275fa660-dirty #79
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x68/0xa0
 __lock_acquire+0x10f2/0x1ca0
 lock_acquire+0xbe/0x2d0
 __mutex_lock+0x6d/0x400
 demote_store+0x244/0x460
 kernfs_fop_write_iter+0x12c/0x1d0
 vfs_write+0x380/0x540
 ksys_write+0x64/0xe0
 do_syscall_64+0xb9/0x1d0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fa61db14887
RSP: 002b:00007ffc56c48358 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007fa61db14887
RDX: 0000000000000002 RSI: 000055a030050220 RDI: 0000000000000001
RBP: 000055a030050220 R08: 00007fa61dbd1460 R09: 000000007fffffff
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000002
R13: 00007fa61dc1b780 R14: 00007fa61dc17600 R15: 00007fa61dc16a00
 </TASK>

Lockdep considers this an AA deadlock because the different resize_lock
mutexes reside in the same lockdep class, but this is a false positive.
Place them in distinct classes to avoid these warnings.

Fixes: 8531fc6f52f5 ("hugetlb: add hugetlb demote page support")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/hugetlb.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Muchun Song July 11, 2024, 8:27 a.m. UTC | #1 
> On Jul 11, 2024, at 15:10, Miaohe Lin <linmiaohe@huawei.com> wrote:
> 
> When tries to demote 1G hugetlb folios, a lockdep warning is observed:
> 
> ============================================
> WARNING: possible recursive locking detected
> 6.10.0-rc6-00452-ga4d0275fa660-dirty #79 Not tainted
> --------------------------------------------
> bash/710 is trying to acquire lock:
> ffffffff8f0a7850 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0x244/0x460
> 
> but task is already holding lock:
> ffffffff8f0a6f48 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0xae/0x460
> 
> other info that might help us debug this:
> Possible unsafe locking scenario:
> 
>       CPU0
>       ----
>  lock(&h->resize_lock);
>  lock(&h->resize_lock);
> 
> *** DEADLOCK ***
> 
> May be due to missing lock nesting notation
> 
> 4 locks held by bash/710:
> #0: ffff8f118439c3f0 (sb_writers#5){.+.+}-{0:0}, at: ksys_write+0x64/0xe0
> #1: ffff8f11893b9e88 (&of->mutex#2){+.+.}-{3:3}, at: kernfs_fop_write_iter+0xf8/0x1d0
> #2: ffff8f1183dc4428 (kn->active#98){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x100/0x1d0
> #3: ffffffff8f0a6f48 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0xae/0x460
> 
> stack backtrace:
> CPU: 3 PID: 710 Comm: bash Not tainted 6.10.0-rc6-00452-ga4d0275fa660-dirty #79
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> Call Trace:
> <TASK>
> dump_stack_lvl+0x68/0xa0
> __lock_acquire+0x10f2/0x1ca0
> lock_acquire+0xbe/0x2d0
> __mutex_lock+0x6d/0x400
> demote_store+0x244/0x460
> kernfs_fop_write_iter+0x12c/0x1d0
> vfs_write+0x380/0x540
> ksys_write+0x64/0xe0
> do_syscall_64+0xb9/0x1d0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7fa61db14887
> RSP: 002b:00007ffc56c48358 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007fa61db14887
> RDX: 0000000000000002 RSI: 000055a030050220 RDI: 0000000000000001
> RBP: 000055a030050220 R08: 00007fa61dbd1460 R09: 000000007fffffff
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000002
> R13: 00007fa61dc1b780 R14: 00007fa61dc17600 R15: 00007fa61dc16a00
> </TASK>
> 
> Lockdep considers this an AA deadlock because the different resize_lock
> mutexes reside in the same lockdep class, but this is a false positive.
> Place them in distinct classes to avoid these warnings.
> 
> Fixes: 8531fc6f52f5 ("hugetlb: add hugetlb demote page support")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> mm/hugetlb.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 45fd3bc75332..2004e6d3f7ca 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4659,6 +4659,8 @@ bool __init __attribute((weak)) arch_hugetlb_valid_size(unsigned long size)
> return size == HPAGE_SIZE;
> }
> 
> +static struct lock_class_key hugetlb_resize_keys[HUGE_MAX_HSTATE];

It's better to let this into "struct hstate".

> +
> void __init hugetlb_add_hstate(unsigned int order)
> {
> 	struct hstate *h;
> @@ -4671,6 +4673,7 @@ void __init hugetlb_add_hstate(unsigned int order)
> 	BUG_ON(order < order_base_2(__NR_USED_SUBPAGE));
> 	h = &hstates[hugetlb_max_hstate++];
> 	mutex_init(&h->resize_lock);

mutex_init() already declares a lock_class_key structure by itself, in
order to avoid this, you should use __mutex_init().

Thanks.

> + 	lockdep_set_class(&h->resize_lock, &hugetlb_resize_keys[hstate_index(h)]);
> 	h->order = order;
> 	h->mask = ~(huge_page_size(h) - 1);
> 	for (i = 0; i < MAX_NUMNODES; ++i)
> -- 
> 2.33.0
>
Miaohe Lin July 12, 2024, 2:08 a.m. UTC | #2 
On 2024/7/11 16:27, Muchun Song wrote:
> 
> 
>> On Jul 11, 2024, at 15:10, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> When tries to demote 1G hugetlb folios, a lockdep warning is observed:
>>
>> ============================================
>> WARNING: possible recursive locking detected
>> 6.10.0-rc6-00452-ga4d0275fa660-dirty #79 Not tainted
>> --------------------------------------------
>> bash/710 is trying to acquire lock:
>> ffffffff8f0a7850 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0x244/0x460
>>
>> but task is already holding lock:
>> ffffffff8f0a6f48 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0xae/0x460
>>
>> other info that might help us debug this:
>> Possible unsafe locking scenario:
>>
>>       CPU0
>>       ----
>>  lock(&h->resize_lock);
>>  lock(&h->resize_lock);
>>
>> *** DEADLOCK ***
>>
>> May be due to missing lock nesting notation
>>
>> 4 locks held by bash/710:
>> #0: ffff8f118439c3f0 (sb_writers#5){.+.+}-{0:0}, at: ksys_write+0x64/0xe0
>> #1: ffff8f11893b9e88 (&of->mutex#2){+.+.}-{3:3}, at: kernfs_fop_write_iter+0xf8/0x1d0
>> #2: ffff8f1183dc4428 (kn->active#98){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x100/0x1d0
>> #3: ffffffff8f0a6f48 (&h->resize_lock){+.+.}-{3:3}, at: demote_store+0xae/0x460
>>
>> stack backtrace:
>> CPU: 3 PID: 710 Comm: bash Not tainted 6.10.0-rc6-00452-ga4d0275fa660-dirty #79
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
>> Call Trace:
>> <TASK>
>> dump_stack_lvl+0x68/0xa0
>> __lock_acquire+0x10f2/0x1ca0
>> lock_acquire+0xbe/0x2d0
>> __mutex_lock+0x6d/0x400
>> demote_store+0x244/0x460
>> kernfs_fop_write_iter+0x12c/0x1d0
>> vfs_write+0x380/0x540
>> ksys_write+0x64/0xe0
>> do_syscall_64+0xb9/0x1d0
>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>> RIP: 0033:0x7fa61db14887
>> RSP: 002b:00007ffc56c48358 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
>> RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007fa61db14887
>> RDX: 0000000000000002 RSI: 000055a030050220 RDI: 0000000000000001
>> RBP: 000055a030050220 R08: 00007fa61dbd1460 R09: 000000007fffffff
>> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000002
>> R13: 00007fa61dc1b780 R14: 00007fa61dc17600 R15: 00007fa61dc16a00
>> </TASK>
>>
>> Lockdep considers this an AA deadlock because the different resize_lock
>> mutexes reside in the same lockdep class, but this is a false positive.
>> Place them in distinct classes to avoid these warnings.
>>
>> Fixes: 8531fc6f52f5 ("hugetlb: add hugetlb demote page support")
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>> mm/hugetlb.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 45fd3bc75332..2004e6d3f7ca 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -4659,6 +4659,8 @@ bool __init __attribute((weak)) arch_hugetlb_valid_size(unsigned long size)
>> return size == HPAGE_SIZE;
>> }
>>
>> +static struct lock_class_key hugetlb_resize_keys[HUGE_MAX_HSTATE];
> 
> It's better to let this into "struct hstate".
> 
>> +
>> void __init hugetlb_add_hstate(unsigned int order)
>> {
>> 	struct hstate *h;
>> @@ -4671,6 +4673,7 @@ void __init hugetlb_add_hstate(unsigned int order)
>> 	BUG_ON(order < order_base_2(__NR_USED_SUBPAGE));
>> 	h = &hstates[hugetlb_max_hstate++];
>> 	mutex_init(&h->resize_lock);
> 
> mutex_init() already declares a lock_class_key structure by itself, in
> order to avoid this, you should use __mutex_init().

While searching the code, I find we can do this in two ways:
1.__mutex_init with separate lock_class_key
2.mutex_init + lockdep_set_class

These are all fine to me. And I will use __mutex_init and move hugetlb_resize_keys
into "struct hstate" as you suggested.
Thanks.
.
diff mbox series

Patch

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 45fd3bc75332..2004e6d3f7ca 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4659,6 +4659,8 @@  bool __init __attribute((weak)) arch_hugetlb_valid_size(unsigned long size)
 	return size == HPAGE_SIZE;
 }
 
+static struct lock_class_key hugetlb_resize_keys[HUGE_MAX_HSTATE];
+
 void __init hugetlb_add_hstate(unsigned int order)
 {
 	struct hstate *h;
@@ -4671,6 +4673,7 @@  void __init hugetlb_add_hstate(unsigned int order)
 	BUG_ON(order < order_base_2(__NR_USED_SUBPAGE));
 	h = &hstates[hugetlb_max_hstate++];
 	mutex_init(&h->resize_lock);
+	lockdep_set_class(&h->resize_lock, &hugetlb_resize_keys[hstate_index(h)]);
 	h->order = order;
 	h->mask = ~(huge_page_size(h) - 1);
 	for (i = 0; i < MAX_NUMNODES; ++i)