The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log [3] in this scenario:
Process 1 Process 2 --- --- hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
This is a ABBA deadlock involving two locks: - Lock A: pagecache_folio lock - Lock B: hugetlb_fault_mutex_table lock
The deadlock occurs between two processes as follows: 1. The first process (let’s call it Process 1) is handling a copy-on-write (COW) operation on a hugepage via hugetlb_wp. Due to insufficient reserved hugetlb pages, Process 1, owner of the reserved hugetlb page, attempts to unmap a hugepage owned by another process (non-owner) to satisfy the reservation. Before unmapping, Process 1 acquires lock B (hugetlb_fault_mutex_table lock) and then lock A (pagecache_folio lock). To proceed with the unmap, it releases Lock B but retains Lock A. After the unmap, Process 1 tries to reacquire Lock B. However, at this point, Lock B has already been acquired by another process.
2. The second process (Process 2) enters the hugetlb_fault handler during the unmap operation. It successfully acquires Lock B (hugetlb_fault_mutex_table lock) that was just released by Process 1, but then attempts to acquire Lock A (pagecache_folio lock), which is still held by Process 1.
As a result, Process 1 (holding Lock A) is blocked waiting for Lock B (held by Process 2), while Process 2 (holding Lock B) is blocked waiting for Lock A (held by Process 1), constructing a ABBA deadlock scenario.
The solution here is to unlock the pagecache_folio and provide the pagecache_folio_unlocked variable to the caller to have the visibility over the pagecache_folio status for subsequent handling.
The error message: INFO: task repro_20250402_:13229 blocked for more than 64 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:25856 pid:13229 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00004006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 schedule_preempt_disabled+0x15/0x30 __mutex_lock+0x75f/0xeb0 hugetlb_wp+0xf88/0x3440 hugetlb_fault+0x14c8/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0x61d/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0010:__put_user_4+0xd/0x20 copy_process+0x1f4a/0x3d60 kernel_clone+0x210/0x8f0 __x64_sys_clone+0x18d/0x1f0 do_syscall_64+0x6a/0x120 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x41b26d </TASK> INFO: task repro_20250402_:13229 is blocked on a mutex likely owned by task repro_20250402_:13250. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK> INFO: task repro_20250402_:13250 blocked for more than 65 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK>
Showing all locks held in the system: 1 lock held by khungtaskd/35: #0: ffffffff879a7440 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x30/0x180 2 locks held by repro_20250402_/13229: #0: ffff888017d801e0 (&mm->mmap_lock){++++}-{4:4}, at: lock_mm_and_find_vma+0x37/0x300 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_wp+0xf88/0x3440 3 locks held by repro_20250402_/13250: #0: ffff8880177f3d08 (vm_lock){++++}-{0:0}, at: do_user_addr_fault+0x41b/0x1490 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_fault+0x3b8/0x2c30 #2: ffff8880129500e8 (&resv_map->rw_sema){++++}-{4:4}, at: hugetlb_fault+0x494/0x2c30
Link: https://drive.google.com/file/d/1DVRnIW-vSayU5J1re9Ct_br3jJQU6Vpb/view?usp=d... [1] Link: https://github.com/bboymimi/bpftracer/blob/master/scripts/hugetlb_lock_debug... [2] Link: https://drive.google.com/file/d/1bWq2-8o-BJAuhoHWX7zAhI6ggfhVzQUI/view?usp=s... [3] Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org Cc: Hugh Dickins hughd@google.com Cc: Florent Revest revest@google.com Cc: Gavin Shan gshan@redhat.com Signed-off-by: Gavin Guo gavinguo@igalia.com --- mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma, * Keep the pte_same checks anyway to make transition from the mutex easier. */ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, - struct vm_fault *vmf) + struct vm_fault *vmf, + bool *pagecache_folio_unlocked) { struct vm_area_struct *vma = vmf->vma; struct mm_struct *mm = vma->vm_mm; @@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash;
folio_put(old_folio); + /* + * The pagecache_folio needs to be unlocked to avoid + * deadlock and we won't re-lock it in hugetlb_wp(). The + * pagecache_folio could be truncated after being + * unlocked. So its state should not be relied + * subsequently. + * + * Setting *pagecache_folio_unlocked to true allows the + * caller to handle any necessary logic related to the + * folio's unlocked state. + */ + if (pagecache_folio) { + folio_unlock(pagecache_folio); + if (pagecache_folio_unlocked) + *pagecache_folio_unlocked = true; + } /* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock @@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */ - ret = hugetlb_wp(folio, vmf); + ret = hugetlb_wp(folio, vmf, NULL); }
spin_unlock(vmf->ptl); @@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0; + bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h), @@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) { - ret = hugetlb_wp(pagecache_folio, &vmf); + ret = hugetlb_wp(pagecache_folio, &vmf, + &pagecache_folio_unlocked); goto out_put_page; } else if (likely(flags & FAULT_FLAG_WRITE)) { vmf.orig_pte = huge_pte_mkdirty(vmf.orig_pte); @@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl);
- if (pagecache_folio) { + /* + * If the pagecache_folio is unlocked in hugetlb_wp(), we skip + * folio_unlock() here. + */ + if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio); + if (pagecache_folio) folio_put(pagecache_folio); - } out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
On Tue, 13 May 2025 17:34:48 +0800 Gavin Guo gavinguo@igalia.com wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log [3] in this scenario:
Process 1 Process 2
hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
This is a ABBA deadlock involving two locks:
- Lock A: pagecache_folio lock
- Lock B: hugetlb_fault_mutex_table lock
Nostalgia. A decade or three ago many of us spent much of our lives staring at ABBA deadlocks. Then came lockdep and after a few more years, it all stopped. I've long hoped that lockdep would gain a solution to custom locks such as folio_wait_bit_common(), but not yet.
Byungchul, please take a look. Would DEPT (https://lkml.kernel.org/r/20250513100730.12664-1-byungchul@sk.com) have warned us about this?
...
The deadlock occurs between two processes as follows:
...
Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org
It's been there for three years so I assume we aren't in a hurry.
The fix looks a bit nasty, sorry. Perhaps designed for a minimal patch footprint? That's good for a backportable fixup, but a more broadly architected solution may be needed going forward.
I'll queue it for 6.16-rc1 with a cc:stable, so this should be presented to the -stable trees 3-4 weeks from now.
On Tue, May 13, 2025 at 05:56:33PM -0700, Andrew Morton wrote:
On Tue, 13 May 2025 17:34:48 +0800 Gavin Guo gavinguo@igalia.com wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log [3] in this scenario:
Process 1 Process 2
hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
This is a ABBA deadlock involving two locks:
- Lock A: pagecache_folio lock
- Lock B: hugetlb_fault_mutex_table lock
Nostalgia. A decade or three ago many of us spent much of our lives staring at ABBA deadlocks. Then came lockdep and after a few more years, it all stopped. I've long hoped that lockdep would gain a solution to custom locks such as folio_wait_bit_common(), but not yet.
Byungchul, please take a look. Would DEPT (https://lkml.kernel.org/r/20250513100730.12664-1-byungchul@sk.com) have warned us about this?
Sure, I will check it. I think this type of deadlock is what DEPT can do the best.
Byungchul
...
The deadlock occurs between two processes as follows:
...
Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org
It's been there for three years so I assume we aren't in a hurry.
The fix looks a bit nasty, sorry. Perhaps designed for a minimal patch footprint? That's good for a backportable fixup, but a more broadly architected solution may be needed going forward.
I'll queue it for 6.16-rc1 with a cc:stable, so this should be presented to the -stable trees 3-4 weeks from now.
On Tue, May 13, 2025 at 05:34:48PM +0800, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log
Hi,
I'm trying to reproduce using the test program [1]. But not yet produced. I see a lot of segfaults while running [1]. I guess something goes wrong. Is there any prerequisite condition to reproduce it? Lemme know if any. Or can you try DEPT15 with your config and environment by the following steps:
1. Apply the patchset on v6.15-rc6. https://lkml.kernel.org/r/20250513100730.12664-1-byungchul@sk.com 2. Turn on CONFIG_DEPT. 3. Run test program reproducing the deadlock. 4. Check dmesg to see if dept reported the dependency.
Byungchul
[3] in this scenario:
Process 1 Process 2
hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
This is a ABBA deadlock involving two locks:
- Lock A: pagecache_folio lock
- Lock B: hugetlb_fault_mutex_table lock
The deadlock occurs between two processes as follows:
- The first process (let’s call it Process 1) is handling a
copy-on-write (COW) operation on a hugepage via hugetlb_wp. Due to insufficient reserved hugetlb pages, Process 1, owner of the reserved hugetlb page, attempts to unmap a hugepage owned by another process (non-owner) to satisfy the reservation. Before unmapping, Process 1 acquires lock B (hugetlb_fault_mutex_table lock) and then lock A (pagecache_folio lock). To proceed with the unmap, it releases Lock B but retains Lock A. After the unmap, Process 1 tries to reacquire Lock B. However, at this point, Lock B has already been acquired by another process.
- The second process (Process 2) enters the hugetlb_fault handler
during the unmap operation. It successfully acquires Lock B (hugetlb_fault_mutex_table lock) that was just released by Process 1, but then attempts to acquire Lock A (pagecache_folio lock), which is still held by Process 1.
As a result, Process 1 (holding Lock A) is blocked waiting for Lock B (held by Process 2), while Process 2 (holding Lock B) is blocked waiting for Lock A (held by Process 1), constructing a ABBA deadlock scenario.
The solution here is to unlock the pagecache_folio and provide the pagecache_folio_unlocked variable to the caller to have the visibility over the pagecache_folio status for subsequent handling.
The error message: INFO: task repro_20250402_:13229 blocked for more than 64 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:25856 pid:13229 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00004006 Call Trace:
<TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 schedule_preempt_disabled+0x15/0x30 __mutex_lock+0x75f/0xeb0 hugetlb_wp+0xf88/0x3440 hugetlb_fault+0x14c8/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0x61d/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0010:__put_user_4+0xd/0x20 copy_process+0x1f4a/0x3d60 kernel_clone+0x210/0x8f0 __x64_sys_clone+0x18d/0x1f0 do_syscall_64+0x6a/0x120 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x41b26d </TASK> INFO: task repro_20250402_:13229 is blocked on a mutex likely owned by task repro_20250402_:13250. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK> INFO: task repro_20250402_:13250 blocked for more than 65 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK>
Showing all locks held in the system: 1 lock held by khungtaskd/35: #0: ffffffff879a7440 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x30/0x180 2 locks held by repro_20250402_/13229: #0: ffff888017d801e0 (&mm->mmap_lock){++++}-{4:4}, at: lock_mm_and_find_vma+0x37/0x300 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_wp+0xf88/0x3440 3 locks held by repro_20250402_/13250: #0: ffff8880177f3d08 (vm_lock){++++}-{0:0}, at: do_user_addr_fault+0x41b/0x1490 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_fault+0x3b8/0x2c30 #2: ffff8880129500e8 (&resv_map->rw_sema){++++}-{4:4}, at: hugetlb_fault+0x494/0x2c30
Link: https://drive.google.com/file/d/1DVRnIW-vSayU5J1re9Ct_br3jJQU6Vpb/view?usp=d... [1] Link: https://github.com/bboymimi/bpftracer/blob/master/scripts/hugetlb_lock_debug... [2] Link: https://drive.google.com/file/d/1bWq2-8o-BJAuhoHWX7zAhI6ggfhVzQUI/view?usp=s... [3] Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org Cc: Hugh Dickins hughd@google.com Cc: Florent Revest revest@google.com Cc: Gavin Shan gshan@redhat.com Signed-off-by: Gavin Guo gavinguo@igalia.com
mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
- Keep the pte_same checks anyway to make transition from the mutex easier.
*/ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio,
struct vm_fault *vmf)
struct vm_fault *vmf,bool *pagecache_folio_unlocked){ struct vm_area_struct *vma = vmf->vma; struct mm_struct *mm = vma->vm_mm; @@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio);
/** The pagecache_folio needs to be unlocked to avoid* deadlock and we won't re-lock it in hugetlb_wp(). The* pagecache_folio could be truncated after being* unlocked. So its state should not be relied* subsequently.** Setting *pagecache_folio_unlocked to true allows the* caller to handle any necessary logic related to the* folio's unlocked state.*/if (pagecache_folio) {folio_unlock(pagecache_folio);if (pagecache_folio_unlocked)*pagecache_folio_unlocked = true;} /* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock@@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */
ret = hugetlb_wp(folio, vmf);
ret = hugetlb_wp(folio, vmf, NULL);spin_unlock(vmf->ptl); @@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0;
- bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h),
@@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) {
ret = hugetlb_wp(pagecache_folio, &vmf);
ret = hugetlb_wp(pagecache_folio, &vmf,&pagecache_folio_unlocked); goto out_put_page;@@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl);
- if (pagecache_folio) {
- /*
* If the pagecache_folio is unlocked in hugetlb_wp(), we skip* folio_unlock() here.*/- if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio);
- if (pagecache_folio) folio_put(pagecache_folio);
- }
out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
2.43.0
Hi Byungchul,
On 5/14/25 14:47, Byungchul Park wrote:
On Tue, May 13, 2025 at 05:34:48PM +0800, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log
Hi,
I'm trying to reproduce using the test program [1]. But not yet produced. I see a lot of segfaults while running [1]. I guess something goes wrong. Is there any prerequisite condition to reproduce it? Lemme know if any. Or can you try DEPT15 with your config and environment by the following steps:
1. Apply the patchset on v6.15-rc6. https://lkml.kernel.org/r/20250513100730.12664-1-byungchul@sk.com 2. Turn on CONFIG_DEPT. 3. Run test program reproducing the deadlock. 4. Check dmesg to see if dept reported the dependency.Byungchul
I have enabled the patchset and successfully reproduced the bug. It seems that there is no warning or error log related to the lock. Did I miss anything? This is the console log: https://drive.google.com/file/d/1dxWNiO71qE-H-e5NMPqj7W-aW5CkGSSF/view?usp=s...
[3] in this scenario:
Process 1 Process 2
hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
This is a ABBA deadlock involving two locks:
- Lock A: pagecache_folio lock
- Lock B: hugetlb_fault_mutex_table lock
The deadlock occurs between two processes as follows:
- The first process (let’s call it Process 1) is handling a
copy-on-write (COW) operation on a hugepage via hugetlb_wp. Due to insufficient reserved hugetlb pages, Process 1, owner of the reserved hugetlb page, attempts to unmap a hugepage owned by another process (non-owner) to satisfy the reservation. Before unmapping, Process 1 acquires lock B (hugetlb_fault_mutex_table lock) and then lock A (pagecache_folio lock). To proceed with the unmap, it releases Lock B but retains Lock A. After the unmap, Process 1 tries to reacquire Lock B. However, at this point, Lock B has already been acquired by another process.
- The second process (Process 2) enters the hugetlb_fault handler
during the unmap operation. It successfully acquires Lock B (hugetlb_fault_mutex_table lock) that was just released by Process 1, but then attempts to acquire Lock A (pagecache_folio lock), which is still held by Process 1.
As a result, Process 1 (holding Lock A) is blocked waiting for Lock B (held by Process 2), while Process 2 (holding Lock B) is blocked waiting for Lock A (held by Process 1), constructing a ABBA deadlock scenario.
The solution here is to unlock the pagecache_folio and provide the pagecache_folio_unlocked variable to the caller to have the visibility over the pagecache_folio status for subsequent handling.
The error message: INFO: task repro_20250402_:13229 blocked for more than 64 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:25856 pid:13229 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00004006 Call Trace:
<TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 schedule_preempt_disabled+0x15/0x30 __mutex_lock+0x75f/0xeb0 hugetlb_wp+0xf88/0x3440 hugetlb_fault+0x14c8/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0x61d/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0010:__put_user_4+0xd/0x20 copy_process+0x1f4a/0x3d60 kernel_clone+0x210/0x8f0 __x64_sys_clone+0x18d/0x1f0 do_syscall_64+0x6a/0x120 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x41b26d </TASK> INFO: task repro_20250402_:13229 is blocked on a mutex likely owned by task repro_20250402_:13250. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK> INFO: task repro_20250402_:13250 blocked for more than 65 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK>
Showing all locks held in the system: 1 lock held by khungtaskd/35: #0: ffffffff879a7440 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x30/0x180 2 locks held by repro_20250402_/13229: #0: ffff888017d801e0 (&mm->mmap_lock){++++}-{4:4}, at: lock_mm_and_find_vma+0x37/0x300 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_wp+0xf88/0x3440 3 locks held by repro_20250402_/13250: #0: ffff8880177f3d08 (vm_lock){++++}-{0:0}, at: do_user_addr_fault+0x41b/0x1490 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_fault+0x3b8/0x2c30 #2: ffff8880129500e8 (&resv_map->rw_sema){++++}-{4:4}, at: hugetlb_fault+0x494/0x2c30
Link: https://drive.google.com/file/d/1DVRnIW-vSayU5J1re9Ct_br3jJQU6Vpb/view?usp=d... [1] Link: https://github.com/bboymimi/bpftracer/blob/master/scripts/hugetlb_lock_debug... [2] Link: https://drive.google.com/file/d/1bWq2-8o-BJAuhoHWX7zAhI6ggfhVzQUI/view?usp=s... [3] Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org Cc: Hugh Dickins hughd@google.com Cc: Florent Revest revest@google.com Cc: Gavin Shan gshan@redhat.com Signed-off-by: Gavin Guo gavinguo@igalia.com
mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
- Keep the pte_same checks anyway to make transition from the mutex easier.
*/ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio,
struct vm_fault *vmf)
struct vm_fault *vmf,bool *pagecache_folio_unlocked)@@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio);
/** The pagecache_folio needs to be unlocked to avoid* deadlock and we won't re-lock it in hugetlb_wp(). The* pagecache_folio could be truncated after being* unlocked. So its state should not be relied* subsequently.** Setting *pagecache_folio_unlocked to true allows the* caller to handle any necessary logic related to the* folio's unlocked state.*/if (pagecache_folio) {folio_unlock(pagecache_folio);if (pagecache_folio_unlocked)*pagecache_folio_unlocked = true;} /* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock@@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */
ret = hugetlb_wp(folio, vmf);
ret = hugetlb_wp(folio, vmf, NULL);spin_unlock(vmf->ptl); @@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0;
- bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h),
@@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) {
ret = hugetlb_wp(pagecache_folio, &vmf);
ret = hugetlb_wp(pagecache_folio, &vmf,&pagecache_folio_unlocked); goto out_put_page;@@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl);
- if (pagecache_folio) {
- /*
* If the pagecache_folio is unlocked in hugetlb_wp(), we skip* folio_unlock() here.*/- if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio);
- if (pagecache_folio) folio_put(pagecache_folio);
- } out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
2.43.0
On Wed, May 14, 2025 at 04:10:12PM +0800, Gavin Guo wrote:
Hi Byungchul,
On 5/14/25 14:47, Byungchul Park wrote:
On Tue, May 13, 2025 at 05:34:48PM +0800, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log
Hi,
I'm trying to reproduce using the test program [1]. But not yet produced. I see a lot of segfaults while running [1]. I guess something goes wrong. Is there any prerequisite condition to reproduce it? Lemme know if any. Or can you try DEPT15 with your config and environment by the following steps:
1. Apply the patchset on v6.15-rc6. https://lkml.kernel.org/r/20250513100730.12664-1-byungchul@sk.com 2. Turn on CONFIG_DEPT. 3. Run test program reproducing the deadlock. 4. Check dmesg to see if dept reported the dependency.Byungchul
I have enabled the patchset and successfully reproduced the bug. It seems that there is no warning or error log related to the lock. Did I miss anything? This is the console log: https://drive.google.com/file/d/1dxWNiO71qE-H-e5NMPqj7W-aW5CkGSSF/view?usp=s...
Hi,
No, you don't miss anything.
[3] in this scenario:
Process 1 Process 2
hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked
^ dept adds B->A dependency.
unmap_ref_private ... mutex_lock(B) // retake and blocked
Since folio_unlock(A) might be called from others than Process 1 and resolve the hang, dept delays the decision adding A->B dependency until dept actually sees folio_unlock(A).
I guess the folio_unlock(A) has never been hit with B held, before the hang. dept would've detected the deadlock in advance if folio_unlock(A) had been hit with B held at least once while testing.
Or to detect that in advance, we can tell dept that the pair of folio_lock() and folio_unlock() happens in the same context like, which looks a bit ugly tho:
folio_lock(A); + /* + * Means the interesting event, folio_unlock(), will appear within + * this context. + */ + sdt_ecxt_enter(&pg_locked_map);
...
+ sdt_ecxt_exit(&pg_locked_map); folio_unlock(A);
Feel free to lemme know if you have any idea for better work.
Byungchul
This is a ABBA deadlock involving two locks:
- Lock A: pagecache_folio lock
- Lock B: hugetlb_fault_mutex_table lock
The deadlock occurs between two processes as follows:
- The first process (let’s call it Process 1) is handling a
copy-on-write (COW) operation on a hugepage via hugetlb_wp. Due to insufficient reserved hugetlb pages, Process 1, owner of the reserved hugetlb page, attempts to unmap a hugepage owned by another process (non-owner) to satisfy the reservation. Before unmapping, Process 1 acquires lock B (hugetlb_fault_mutex_table lock) and then lock A (pagecache_folio lock). To proceed with the unmap, it releases Lock B but retains Lock A. After the unmap, Process 1 tries to reacquire Lock B. However, at this point, Lock B has already been acquired by another process.
- The second process (Process 2) enters the hugetlb_fault handler
during the unmap operation. It successfully acquires Lock B (hugetlb_fault_mutex_table lock) that was just released by Process 1, but then attempts to acquire Lock A (pagecache_folio lock), which is still held by Process 1.
As a result, Process 1 (holding Lock A) is blocked waiting for Lock B (held by Process 2), while Process 2 (holding Lock B) is blocked waiting for Lock A (held by Process 1), constructing a ABBA deadlock scenario.
The solution here is to unlock the pagecache_folio and provide the pagecache_folio_unlocked variable to the caller to have the visibility over the pagecache_folio status for subsequent handling.
The error message: INFO: task repro_20250402_:13229 blocked for more than 64 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:25856 pid:13229 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00004006 Call Trace:
<TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 schedule_preempt_disabled+0x15/0x30 __mutex_lock+0x75f/0xeb0 hugetlb_wp+0xf88/0x3440 hugetlb_fault+0x14c8/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0x61d/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0010:__put_user_4+0xd/0x20 copy_process+0x1f4a/0x3d60 kernel_clone+0x210/0x8f0 __x64_sys_clone+0x18d/0x1f0 do_syscall_64+0x6a/0x120 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x41b26d </TASK> INFO: task repro_20250402_:13229 is blocked on a mutex likely owned by task repro_20250402_:13250. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK> INFO: task repro_20250402_:13250 blocked for more than 65 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK>
Showing all locks held in the system: 1 lock held by khungtaskd/35: #0: ffffffff879a7440 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x30/0x180 2 locks held by repro_20250402_/13229: #0: ffff888017d801e0 (&mm->mmap_lock){++++}-{4:4}, at: lock_mm_and_find_vma+0x37/0x300 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_wp+0xf88/0x3440 3 locks held by repro_20250402_/13250: #0: ffff8880177f3d08 (vm_lock){++++}-{0:0}, at: do_user_addr_fault+0x41b/0x1490 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_fault+0x3b8/0x2c30 #2: ffff8880129500e8 (&resv_map->rw_sema){++++}-{4:4}, at: hugetlb_fault+0x494/0x2c30
Link: https://drive.google.com/file/d/1DVRnIW-vSayU5J1re9Ct_br3jJQU6Vpb/view?usp=d... [1] Link: https://github.com/bboymimi/bpftracer/blob/master/scripts/hugetlb_lock_debug... [2] Link: https://drive.google.com/file/d/1bWq2-8o-BJAuhoHWX7zAhI6ggfhVzQUI/view?usp=s... [3] Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org Cc: Hugh Dickins hughd@google.com Cc: Florent Revest revest@google.com Cc: Gavin Shan gshan@redhat.com Signed-off-by: Gavin Guo gavinguo@igalia.com
mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
- Keep the pte_same checks anyway to make transition from the mutex easier.
*/ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio,
struct vm_fault *vmf)
struct vm_fault *vmf,bool *pagecache_folio_unlocked)@@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio);
/** The pagecache_folio needs to be unlocked to avoid* deadlock and we won't re-lock it in hugetlb_wp(). The* pagecache_folio could be truncated after being* unlocked. So its state should not be relied* subsequently.** Setting *pagecache_folio_unlocked to true allows the* caller to handle any necessary logic related to the* folio's unlocked state.*/if (pagecache_folio) {folio_unlock(pagecache_folio);if (pagecache_folio_unlocked)*pagecache_folio_unlocked = true;} /* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock@@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */
ret = hugetlb_wp(folio, vmf);
ret = hugetlb_wp(folio, vmf, NULL);@@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0;
- bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h),
@@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) {
ret = hugetlb_wp(pagecache_folio, &vmf);
ret = hugetlb_wp(pagecache_folio, &vmf,&pagecache_folio_unlocked); goto out_put_page;@@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl);
- if (pagecache_folio) {
- /*
* If the pagecache_folio is unlocked in hugetlb_wp(), we skip* folio_unlock() here.*/- if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio);
- if (pagecache_folio) folio_put(pagecache_folio);
- } out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
2.43.0
On Wed, May 14, 2025 at 04:10:12PM +0800, Gavin Guo wrote:
Hi Byungchul,
On 5/14/25 14:47, Byungchul Park wrote:
On Tue, May 13, 2025 at 05:34:48PM +0800, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log
Hi,
I'm trying to reproduce using the test program [1]. But not yet produced. I see a lot of segfaults while running [1]. I guess something goes wrong. Is there any prerequisite condition to reproduce it? Lemme know if any. Or can you try DEPT15 with your config and environment by the following steps:
1. Apply the patchset on v6.15-rc6. https://lkml.kernel.org/r/20250513100730.12664-1-byungchul@sk.com 2. Turn on CONFIG_DEPT. 3. Run test program reproducing the deadlock. 4. Check dmesg to see if dept reported the dependency.Byungchul
I have enabled the patchset and successfully reproduced the bug. It seems that there is no warning or error log related to the lock. Did I miss anything? This is the console log: https://drive.google.com/file/d/1dxWNiO71qE-H-e5NMPqj7W-aW5CkGSSF/view?usp=s...
My bad. I think I found the problem that dept didn't report it. You might see the report with the following patch applied on the top, there might be a lot of false positives along with that might be annoying tho.
Some of my efforts to suppress false positives, suppressed the real one.
Do you mind if I ask you to run the test with the following patch applied? It'd be appreciated if you do and share the result with me.
Byungchul
--- diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index f31cd68f2935..fd7559e663c5 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -1138,6 +1138,7 @@ static inline bool trylock_page(struct page *page) static inline void folio_lock(struct folio *folio) { might_sleep(); + dept_page_wait_on_bit(&folio->page, PG_locked); if (!folio_trylock(folio)) __folio_lock(folio); } diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c index b2fa96d984bc..4e96a6a72d02 100644 --- a/kernel/dependency/dept.c +++ b/kernel/dependency/dept.c @@ -931,7 +931,6 @@ static void print_circle(struct dept_class *c) dept_outworld_exit();
do { - tc->reported = true; tc = fc; fc = fc->bfs_parent; } while (tc != c); diff --git a/kernel/dependency/dept_unit_test.c b/kernel/dependency/dept_unit_test.c index 88e846b9f876..496149f31fb3 100644 --- a/kernel/dependency/dept_unit_test.c +++ b/kernel/dependency/dept_unit_test.c @@ -125,6 +125,8 @@ static int __init dept_ut_init(void) { int i;
+ return 0; + lockdep_off();
dept_ut_results.ecxt_stack_valid_cnt = 0; --
On 5/16/25 14:03, Byungchul Park wrote:
On Wed, May 14, 2025 at 04:10:12PM +0800, Gavin Guo wrote:
Hi Byungchul,
On 5/14/25 14:47, Byungchul Park wrote:
On Tue, May 13, 2025 at 05:34:48PM +0800, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log
Hi,
I'm trying to reproduce using the test program [1]. But not yet produced. I see a lot of segfaults while running [1]. I guess something goes wrong. Is there any prerequisite condition to reproduce it? Lemme know if any. Or can you try DEPT15 with your config and environment by the following steps:
1. Apply the patchset on v6.15-rc6. https://lkml.kernel.org/r/20250513100730.12664-1-byungchul@sk.com 2. Turn on CONFIG_DEPT. 3. Run test program reproducing the deadlock. 4. Check dmesg to see if dept reported the dependency.Byungchul
I have enabled the patchset and successfully reproduced the bug. It seems that there is no warning or error log related to the lock. Did I miss anything? This is the console log: https://drive.google.com/file/d/1dxWNiO71qE-H-e5NMPqj7W-aW5CkGSSF/view?usp=s...
My bad. I think I found the problem that dept didn't report it. You might see the report with the following patch applied on the top, there might be a lot of false positives along with that might be annoying tho.
Some of my efforts to suppress false positives, suppressed the real one.
Do you mind if I ask you to run the test with the following patch applied? It'd be appreciated if you do and share the result with me.
Byungchul
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index f31cd68f2935..fd7559e663c5 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -1138,6 +1138,7 @@ static inline bool trylock_page(struct page *page) static inline void folio_lock(struct folio *folio) { might_sleep();
- dept_page_wait_on_bit(&folio->page, PG_locked); if (!folio_trylock(folio)) __folio_lock(folio); }
diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c index b2fa96d984bc..4e96a6a72d02 100644 --- a/kernel/dependency/dept.c +++ b/kernel/dependency/dept.c @@ -931,7 +931,6 @@ static void print_circle(struct dept_class *c) dept_outworld_exit(); do {
tc->reported = true;diff --git a/kernel/dependency/dept_unit_test.c b/kernel/dependency/dept_unit_test.c index 88e846b9f876..496149f31fb3 100644 --- a/kernel/dependency/dept_unit_test.c +++ b/kernel/dependency/dept_unit_test.c @@ -125,6 +125,8 @@ static int __init dept_ut_init(void) { int i;
- return 0;
- lockdep_off();
dept_ut_results.ecxt_stack_valid_cnt = 0; --
Please see the test result: https://drive.google.com/file/d/1B20Gu3wLFbAeaXXb7aSQP5T6aeN9Mext/view?usp=s...
It seems that after the first round, the deadlock is captured: ubuntu@localhost:~$ ./repro_20250402_0225_154f8fb0580000 executing program [ 80.425842][ T3416] =================================================== [ 80.426707][ T3416] DEPT: Circular dependency has been detected. [ 80.427497][ T3416] 6.15.0-rc6+ #31 Not tainted [ 80.428084][ T3416] --------------------------------------------------- [ 80.428964][ T3416] summary [ 80.429330][ T3416] --------------------------------------------------- [ 80.430078][ T3416] *** DEADLOCK *** [ 80.430078][ T3416] [ 80.430736][ T3416] context A [ 80.431076][ T3416] [S] (unknown)(pg_locked_map:0) [ 80.431637][ T3416] [W] lock(&hugetlb_fault_mutex_table[i]:0) [ 80.432312][ T3416] [E] dept_page_clear_bit(pg_locked_map:0) [ 80.432977][ T3416] [ 80.433246][ T3416] context B [ 80.433595][ T3416] [S] lock(&hugetlb_fault_mutex_table[i]:0) [ 80.434245][ T3416] [W] dept_page_wait_on_bit(pg_locked_map:0) [ 80.434880][ T3416] [E] unlock(&hugetlb_fault_mutex_table[i]:0) [ 80.435592][ T3416] [ 80.435852][ T3416] [S]: start of the event context [ 80.436369][ T3416] [W]: the wait blocked [ 80.436789][ T3416] [E]: the event not reachable [ 80.437275][ T3416] --------------------------------------------------- [ 80.437950][ T3416] context A's detail [ 80.438367][ T3416] --------------------------------------------------- [ 80.439006][ T3416] context A [ 80.439337][ T3416] [S] (unknown)(pg_locked_map:0) [ 80.439883][ T3416] [W] lock(&hugetlb_fault_mutex_table[i]:0) [ 80.440489][ T3416] [E] dept_page_clear_bit(pg_locked_map:0) [ 80.441075][ T3416] [ 80.441318][ T3416] [S] (unknown)(pg_locked_map:0): [ 80.441816][ T3416] (N/A) [ 80.442077][ T3416] [ 80.442309][ T3416] [W] lock(&hugetlb_fault_mutex_table[i]:0): [ 80.442872][ T3416] [<ffffffff82144644>] hugetlb_wp+0xfa4/0x3490 [ 80.443502][ T3416] stacktrace: [ 80.443810][ T3416] hugetlb_wp+0xfa4/0x3490 [ 80.444267][ T3416] hugetlb_fault+0x1505/0x2c70 [ 80.444776][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.445275][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.445779][ T3416] exc_page_fault+0x67/0x110 [ 80.446239][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.446722][ T3416] __put_user_4+0xd/0x20 [ 80.447157][ T3416] copy_process+0x1f64/0x3d80 [ 80.447621][ T3416] kernel_clone+0x216/0x940 [ 80.448068][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.448548][ T3416] do_syscall_64+0x6f/0x120 [ 80.448999][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.449556][ T3416] [ 80.449765][ T3416] [E] dept_page_clear_bit(pg_locked_map:0): [ 80.450272][ T3416] [<ffffffff8214263b>] hugetlb_fault+0x1ccb/0x2c70 [ 80.450861][ T3416] stacktrace: [ 80.451148][ T3416] hugetlb_fault+0x1ccb/0x2c70 [ 80.451611][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.452080][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.452566][ T3416] exc_page_fault+0x67/0x110 [ 80.453014][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.453497][ T3416] __put_user_4+0xd/0x20 [ 80.453923][ T3416] copy_process+0x1f64/0x3d80 [ 80.454379][ T3416] kernel_clone+0x216/0x940 [ 80.454817][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.455277][ T3416] do_syscall_64+0x6f/0x120 [ 80.455722][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.456253][ T3416] --------------------------------------------------- [ 80.456842][ T3416] context B's detail [ 80.457198][ T3416] --------------------------------------------------- [ 80.457842][ T3416] context B [ 80.458122][ T3416] [S] lock(&hugetlb_fault_mutex_table[i]:0) [ 80.458661][ T3416] [W] dept_page_wait_on_bit(pg_locked_map:0) [ 80.459187][ T3416] [E] unlock(&hugetlb_fault_mutex_table[i]:0) [ 80.459763][ T3416] [ 80.459988][ T3416] [S] lock(&hugetlb_fault_mutex_table[i]:0): [ 80.460509][ T3416] [<ffffffff82140d36>] hugetlb_fault+0x3c6/0x2c70 [ 80.461074][ T3416] stacktrace: [ 80.461374][ T3416] hugetlb_fault+0x3c6/0x2c70 [ 80.461812][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.462281][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.462775][ T3416] exc_page_fault+0x67/0x110 [ 80.463220][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.463694][ T3416] __put_user_4+0xd/0x20 [ 80.464129][ T3416] copy_process+0x1f64/0x3d80 [ 80.464577][ T3416] kernel_clone+0x216/0x940 [ 80.464994][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.465466][ T3416] do_syscall_64+0x6f/0x120 [ 80.465909][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.466457][ T3416] [ 80.466660][ T3416] [W] dept_page_wait_on_bit(pg_locked_map:0): [ 80.467177][ T3416] [<ffffffff82141187>] hugetlb_fault+0x817/0x2c70 [ 80.467740][ T3416] stacktrace: [ 80.468032][ T3416] hugetlb_fault+0x817/0x2c70 [ 80.468479][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.468947][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.469428][ T3416] exc_page_fault+0x67/0x110 [ 80.469865][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.470332][ T3416] __put_user_4+0xd/0x20 [ 80.470742][ T3416] copy_process+0x1f64/0x3d80 [ 80.471186][ T3416] kernel_clone+0x216/0x940 [ 80.471616][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.472060][ T3416] do_syscall_64+0x6f/0x120 [ 80.472492][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.473040][ T3416] [ 80.473271][ T3416] [E] unlock(&hugetlb_fault_mutex_table[i]:0): [ 80.473863][ T3416] (N/A) [ 80.474124][ T3416] --------------------------------------------------- [ 80.474738][ T3416] information that might be helpful [ 80.475210][ T3416] --------------------------------------------------- [ 80.475820][ T3416] CPU: 1 UID: 1000 PID: 3416 Comm: repro_20250402_ Not tainted 6.15.0-rc6+ #31 NONE [ 80.475831][ T3416] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 80.475837][ T3416] Call Trace: [ 80.475841][ T3416] <TASK> [ 80.475845][ T3416] dump_stack_lvl+0x1ad/0x280 [ 80.475858][ T3416] ? __pfx_dump_stack_lvl+0x10/0x10 [ 80.475867][ T3416] ? __pfx__printk+0x10/0x10 [ 80.475883][ T3416] cb_check_dl+0x24a8/0x2530 [ 80.475897][ T3416] ? bfs_extend_dep+0x271/0x290 [ 80.475909][ T3416] bfs+0x464/0x5e0 [ 80.475921][ T3416] ? __pfx_bfs+0x10/0x10 [ 80.475931][ T3416] ? add_dep+0x387/0x710 [ 80.475943][ T3416] add_dep+0x3d0/0x710 [ 80.475953][ T3416] ? __pfx_from_pool+0x10/0x10 [ 80.475963][ T3416] ? __pfx_bfs_init_check_dl+0x10/0x10 [ 80.475972][ T3416] ? __pfx_bfs_extend_dep+0x10/0x10 [ 80.475981][ T3416] ? __pfx_bfs_dequeue_dep+0x10/0x10 [ 80.475990][ T3416] ? __pfx_cb_check_dl+0x10/0x10 [ 80.475999][ T3416] ? __pfx_add_dep+0x10/0x10 [ 80.476011][ T3416] ? put_ecxt+0xda/0x4b0 [ 80.476024][ T3416] __dept_event+0xee8/0x1590 [ 80.476038][ T3416] dept_event+0x166/0x240 [ 80.476047][ T3416] ? hugetlb_fault+0x1ccb/0x2c70 [ 80.476057][ T3416] folio_unlock+0xb8/0x190 [ 80.476071][ T3416] hugetlb_fault+0x1ccb/0x2c70 [ 80.476085][ T3416] ? __pfx_hugetlb_fault+0x10/0x10 [ 80.476100][ T3416] ? mt_find+0x15a/0x5f0 [ 80.476110][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.476125][ T3416] ? handle_mm_fault+0xdb/0x1ab0 [ 80.476142][ T3416] ? __pfx_handle_mm_fault+0x10/0x10 [ 80.476156][ T3416] ? find_vma+0xec/0x160 [ 80.476164][ T3416] ? __pfx_find_vma+0x10/0x10 [ 80.476172][ T3416] ? dept_on+0x1c/0x30 [ 80.476179][ T3416] ? dept_exit+0x1c5/0x2c0 [ 80.476186][ T3416] ? lockdep_hardirqs_on_prepare+0x21/0x280 [ 80.476197][ T3416] ? lock_mm_and_find_vma+0xa1/0x300 [ 80.476211][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.476219][ T3416] ? mntput_no_expire+0xc0/0x870 [ 80.476235][ T3416] ? __pfx_do_user_addr_fault+0x10/0x10 [ 80.476246][ T3416] ? trace_irq_disable+0x60/0x180 [ 80.476258][ T3416] exc_page_fault+0x67/0x110 [ 80.476272][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.476280][ T3416] RIP: 0010:__put_user_4+0xd/0x20 [ 80.476293][ T3416] Code: 66 89 01 31 c9 0f 1f 00 c3 cc cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 48 89 cb 48 c1 fb 3f 48 09 d9 0f 1f 00 <89> 01 31 c9 0 [ 80.476312][ T3416] RSP: 0018:ffffc90004dffa38 EFLAGS: 00010206 [ 80.476322][ T3416] RAX: 000000000000000c RBX: 0000000000000000 RCX: 0000200000000200 [ 80.476329][ T3416] RDX: 0000000000000000 RSI: ffff888016abe300 RDI: ffff888017878c20 [ 80.476335][ T3416] RBP: ffffc90004dffc10 R08: 0000000000000000 R09: 0000000000000000 [ 80.476340][ T3416] R10: 0000000000000000 R11: ffffffff82034b65 R12: ffff888017c0a1e8 [ 80.476346][ T3416] R13: ffff88800d6a8200 R14: 0000000000000000 R15: ffff888017c08a38 [ 80.476354][ T3416] ? __might_fault+0xb5/0x130 [ 80.476367][ T3416] copy_process+0x1f64/0x3d80 [ 80.476375][ T3416] ? lockdep_hardirqs_on_prepare+0x21/0x280 [ 80.476388][ T3416] ? copy_process+0x996/0x3d80 [ 80.476399][ T3416] ? __pfx_copy_process+0x10/0x10 [ 80.476406][ T3416] ? from_pool+0x1e1/0x750 [ 80.476416][ T3416] ? handle_mm_fault+0x122e/0x1ab0 [ 80.476432][ T3416] kernel_clone+0x216/0x940 [ 80.476440][ T3416] ? __pfx_llist_del_first+0x10/0x10 [ 80.476448][ T3416] ? check_new_class+0x28a/0xe90 [ 80.476458][ T3416] ? __pfx_kernel_clone+0x10/0x10 [ 80.476468][ T3416] ? from_pool+0x1e1/0x750 [ 80.476478][ T3416] ? __pfx_from_pool+0x10/0x10 [ 80.476487][ T3416] ? __pfx_from_pool+0x10/0x10 [ 80.476502][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.476512][ T3416] ? __pfx___x64_sys_clone+0x10/0x10 [ 80.476520][ T3416] ? llist_add_batch+0x111/0x1f0 [ 80.476532][ T3416] ? dept_task+0x5/0x20 [ 80.476539][ T3416] ? dept_on+0x1c/0x30 [ 80.476545][ T3416] ? dept_exit+0x1c5/0x2c0 [ 80.476553][ T3416] ? lockdep_hardirqs_on_prepare+0x21/0x280 [ 80.476565][ T3416] do_syscall_64+0x6f/0x120 [ 80.476573][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.476580][ T3416] RIP: 0033:0x41b26d [ 80.476588][ T3416] Code: b3 66 2e 0f 1f 84 00 00 00 00 00 66 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 8 [ 80.476595][ T3416] RSP: 002b:00007ffa1ad2d198 EFLAGS: 00000206 ORIG_RAX: 0000000000000038 [ 80.476604][ T3416] RAX: ffffffffffffffda RBX: 00007ffa1ad2dcdc RCX: 000000000041b26d [ 80.476610][ T3416] RDX: 0000200000000200 RSI: 0000000000000000 RDI: 0000000000001200 [ 80.476616][ T3416] RBP: 00007ffa1ad2d1e0 R08: 0000000000000000 R09: 0000000000000000 [ 80.476621][ T3416] R10: 0000000000000000 R11: 0000000000000206 R12: 00007ffa1ad2d6c0 [ 80.476626][ T3416] R13: ffffffffffffffb8 R14: 0000000000000002 R15: 00007ffd95d76940 [ 80.476638][ T3416] </TASK>
On Fri, May 16, 2025 at 03:32:35PM +0800, Gavin Guo wrote:
On 5/16/25 14:03, Byungchul Park wrote:
On Wed, May 14, 2025 at 04:10:12PM +0800, Gavin Guo wrote:
Hi Byungchul,
On 5/14/25 14:47, Byungchul Park wrote:
On Tue, May 13, 2025 at 05:34:48PM +0800, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log
Hi,
I'm trying to reproduce using the test program [1]. But not yet produced. I see a lot of segfaults while running [1]. I guess something goes wrong. Is there any prerequisite condition to reproduce it? Lemme know if any. Or can you try DEPT15 with your config and environment by the following steps:
1. Apply the patchset on v6.15-rc6. https://lkml.kernel.org/r/20250513100730.12664-1-byungchul@sk.com 2. Turn on CONFIG_DEPT. 3. Run test program reproducing the deadlock. 4. Check dmesg to see if dept reported the dependency.Byungchul
I have enabled the patchset and successfully reproduced the bug. It seems that there is no warning or error log related to the lock. Did I miss anything? This is the console log: https://drive.google.com/file/d/1dxWNiO71qE-H-e5NMPqj7W-aW5CkGSSF/view?usp=s...
My bad. I think I found the problem that dept didn't report it. You might see the report with the following patch applied on the top, there might be a lot of false positives along with that might be annoying tho.
Some of my efforts to suppress false positives, suppressed the real one.
Do you mind if I ask you to run the test with the following patch applied? It'd be appreciated if you do and share the result with me.
Byungchul
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index f31cd68f2935..fd7559e663c5 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -1138,6 +1138,7 @@ static inline bool trylock_page(struct page *page) static inline void folio_lock(struct folio *folio) { might_sleep();
- dept_page_wait_on_bit(&folio->page, PG_locked); if (!folio_trylock(folio)) __folio_lock(folio); }
diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c index b2fa96d984bc..4e96a6a72d02 100644 --- a/kernel/dependency/dept.c +++ b/kernel/dependency/dept.c @@ -931,7 +931,6 @@ static void print_circle(struct dept_class *c) dept_outworld_exit(); do {
tc->reported = true;diff --git a/kernel/dependency/dept_unit_test.c b/kernel/dependency/dept_unit_test.c index 88e846b9f876..496149f31fb3 100644 --- a/kernel/dependency/dept_unit_test.c +++ b/kernel/dependency/dept_unit_test.c @@ -125,6 +125,8 @@ static int __init dept_ut_init(void) { int i;
- return 0;
- lockdep_off(); dept_ut_results.ecxt_stack_valid_cnt = 0;
--
Please see the test result: https://drive.google.com/file/d/1B20Gu3wLFbAeaXXb7aSQP5T6aeN9Mext/view?usp=s...
It seems that after the first round, the deadlock is captured:
Thank you for the testing again!
Yeah, dept works well as I expected. I shouldn't have suppressed dept reports too aggressively, but.. I (or we if any) need to deal with the existing false positives one by one by using dept annotations.
Thanks again for confirming it.
Byungchul
ubuntu@localhost:~$ ./repro_20250402_0225_154f8fb0580000 executing program [ 80.425842][ T3416] =================================================== [ 80.426707][ T3416] DEPT: Circular dependency has been detected. [ 80.427497][ T3416] 6.15.0-rc6+ #31 Not tainted [ 80.428084][ T3416] --------------------------------------------------- [ 80.428964][ T3416] summary [ 80.429330][ T3416] --------------------------------------------------- [ 80.430078][ T3416] *** DEADLOCK *** [ 80.430078][ T3416] [ 80.430736][ T3416] context A [ 80.431076][ T3416] [S] (unknown)(pg_locked_map:0) [ 80.431637][ T3416] [W] lock(&hugetlb_fault_mutex_table[i]:0) [ 80.432312][ T3416] [E] dept_page_clear_bit(pg_locked_map:0) [ 80.432977][ T3416] [ 80.433246][ T3416] context B [ 80.433595][ T3416] [S] lock(&hugetlb_fault_mutex_table[i]:0) [ 80.434245][ T3416] [W] dept_page_wait_on_bit(pg_locked_map:0) [ 80.434880][ T3416] [E] unlock(&hugetlb_fault_mutex_table[i]:0) [ 80.435592][ T3416] [ 80.435852][ T3416] [S]: start of the event context [ 80.436369][ T3416] [W]: the wait blocked [ 80.436789][ T3416] [E]: the event not reachable [ 80.437275][ T3416] --------------------------------------------------- [ 80.437950][ T3416] context A's detail [ 80.438367][ T3416] --------------------------------------------------- [ 80.439006][ T3416] context A [ 80.439337][ T3416] [S] (unknown)(pg_locked_map:0) [ 80.439883][ T3416] [W] lock(&hugetlb_fault_mutex_table[i]:0) [ 80.440489][ T3416] [E] dept_page_clear_bit(pg_locked_map:0) [ 80.441075][ T3416] [ 80.441318][ T3416] [S] (unknown)(pg_locked_map:0): [ 80.441816][ T3416] (N/A) [ 80.442077][ T3416] [ 80.442309][ T3416] [W] lock(&hugetlb_fault_mutex_table[i]:0): [ 80.442872][ T3416] [<ffffffff82144644>] hugetlb_wp+0xfa4/0x3490 [ 80.443502][ T3416] stacktrace: [ 80.443810][ T3416] hugetlb_wp+0xfa4/0x3490 [ 80.444267][ T3416] hugetlb_fault+0x1505/0x2c70 [ 80.444776][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.445275][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.445779][ T3416] exc_page_fault+0x67/0x110 [ 80.446239][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.446722][ T3416] __put_user_4+0xd/0x20 [ 80.447157][ T3416] copy_process+0x1f64/0x3d80 [ 80.447621][ T3416] kernel_clone+0x216/0x940 [ 80.448068][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.448548][ T3416] do_syscall_64+0x6f/0x120 [ 80.448999][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.449556][ T3416] [ 80.449765][ T3416] [E] dept_page_clear_bit(pg_locked_map:0): [ 80.450272][ T3416] [<ffffffff8214263b>] hugetlb_fault+0x1ccb/0x2c70 [ 80.450861][ T3416] stacktrace: [ 80.451148][ T3416] hugetlb_fault+0x1ccb/0x2c70 [ 80.451611][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.452080][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.452566][ T3416] exc_page_fault+0x67/0x110 [ 80.453014][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.453497][ T3416] __put_user_4+0xd/0x20 [ 80.453923][ T3416] copy_process+0x1f64/0x3d80 [ 80.454379][ T3416] kernel_clone+0x216/0x940 [ 80.454817][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.455277][ T3416] do_syscall_64+0x6f/0x120 [ 80.455722][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.456253][ T3416] --------------------------------------------------- [ 80.456842][ T3416] context B's detail [ 80.457198][ T3416] --------------------------------------------------- [ 80.457842][ T3416] context B [ 80.458122][ T3416] [S] lock(&hugetlb_fault_mutex_table[i]:0) [ 80.458661][ T3416] [W] dept_page_wait_on_bit(pg_locked_map:0) [ 80.459187][ T3416] [E] unlock(&hugetlb_fault_mutex_table[i]:0) [ 80.459763][ T3416] [ 80.459988][ T3416] [S] lock(&hugetlb_fault_mutex_table[i]:0): [ 80.460509][ T3416] [<ffffffff82140d36>] hugetlb_fault+0x3c6/0x2c70 [ 80.461074][ T3416] stacktrace: [ 80.461374][ T3416] hugetlb_fault+0x3c6/0x2c70 [ 80.461812][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.462281][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.462775][ T3416] exc_page_fault+0x67/0x110 [ 80.463220][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.463694][ T3416] __put_user_4+0xd/0x20 [ 80.464129][ T3416] copy_process+0x1f64/0x3d80 [ 80.464577][ T3416] kernel_clone+0x216/0x940 [ 80.464994][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.465466][ T3416] do_syscall_64+0x6f/0x120 [ 80.465909][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.466457][ T3416] [ 80.466660][ T3416] [W] dept_page_wait_on_bit(pg_locked_map:0): [ 80.467177][ T3416] [<ffffffff82141187>] hugetlb_fault+0x817/0x2c70 [ 80.467740][ T3416] stacktrace: [ 80.468032][ T3416] hugetlb_fault+0x817/0x2c70 [ 80.468479][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.468947][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.469428][ T3416] exc_page_fault+0x67/0x110 [ 80.469865][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.470332][ T3416] __put_user_4+0xd/0x20 [ 80.470742][ T3416] copy_process+0x1f64/0x3d80 [ 80.471186][ T3416] kernel_clone+0x216/0x940 [ 80.471616][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.472060][ T3416] do_syscall_64+0x6f/0x120 [ 80.472492][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.473040][ T3416] [ 80.473271][ T3416] [E] unlock(&hugetlb_fault_mutex_table[i]:0): [ 80.473863][ T3416] (N/A) [ 80.474124][ T3416] --------------------------------------------------- [ 80.474738][ T3416] information that might be helpful [ 80.475210][ T3416] --------------------------------------------------- [ 80.475820][ T3416] CPU: 1 UID: 1000 PID: 3416 Comm: repro_20250402_ Not tainted 6.15.0-rc6+ #31 NONE [ 80.475831][ T3416] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 80.475837][ T3416] Call Trace: [ 80.475841][ T3416] <TASK> [ 80.475845][ T3416] dump_stack_lvl+0x1ad/0x280 [ 80.475858][ T3416] ? __pfx_dump_stack_lvl+0x10/0x10 [ 80.475867][ T3416] ? __pfx__printk+0x10/0x10 [ 80.475883][ T3416] cb_check_dl+0x24a8/0x2530 [ 80.475897][ T3416] ? bfs_extend_dep+0x271/0x290 [ 80.475909][ T3416] bfs+0x464/0x5e0 [ 80.475921][ T3416] ? __pfx_bfs+0x10/0x10 [ 80.475931][ T3416] ? add_dep+0x387/0x710 [ 80.475943][ T3416] add_dep+0x3d0/0x710 [ 80.475953][ T3416] ? __pfx_from_pool+0x10/0x10 [ 80.475963][ T3416] ? __pfx_bfs_init_check_dl+0x10/0x10 [ 80.475972][ T3416] ? __pfx_bfs_extend_dep+0x10/0x10 [ 80.475981][ T3416] ? __pfx_bfs_dequeue_dep+0x10/0x10 [ 80.475990][ T3416] ? __pfx_cb_check_dl+0x10/0x10 [ 80.475999][ T3416] ? __pfx_add_dep+0x10/0x10 [ 80.476011][ T3416] ? put_ecxt+0xda/0x4b0 [ 80.476024][ T3416] __dept_event+0xee8/0x1590 [ 80.476038][ T3416] dept_event+0x166/0x240 [ 80.476047][ T3416] ? hugetlb_fault+0x1ccb/0x2c70 [ 80.476057][ T3416] folio_unlock+0xb8/0x190 [ 80.476071][ T3416] hugetlb_fault+0x1ccb/0x2c70 [ 80.476085][ T3416] ? __pfx_hugetlb_fault+0x10/0x10 [ 80.476100][ T3416] ? mt_find+0x15a/0x5f0 [ 80.476110][ T3416] handle_mm_fault+0x1845/0x1ab0 [ 80.476125][ T3416] ? handle_mm_fault+0xdb/0x1ab0 [ 80.476142][ T3416] ? __pfx_handle_mm_fault+0x10/0x10 [ 80.476156][ T3416] ? find_vma+0xec/0x160 [ 80.476164][ T3416] ? __pfx_find_vma+0x10/0x10 [ 80.476172][ T3416] ? dept_on+0x1c/0x30 [ 80.476179][ T3416] ? dept_exit+0x1c5/0x2c0 [ 80.476186][ T3416] ? lockdep_hardirqs_on_prepare+0x21/0x280 [ 80.476197][ T3416] ? lock_mm_and_find_vma+0xa1/0x300 [ 80.476211][ T3416] do_user_addr_fault+0x637/0x1450 [ 80.476219][ T3416] ? mntput_no_expire+0xc0/0x870 [ 80.476235][ T3416] ? __pfx_do_user_addr_fault+0x10/0x10 [ 80.476246][ T3416] ? trace_irq_disable+0x60/0x180 [ 80.476258][ T3416] exc_page_fault+0x67/0x110 [ 80.476272][ T3416] asm_exc_page_fault+0x26/0x30 [ 80.476280][ T3416] RIP: 0010:__put_user_4+0xd/0x20 [ 80.476293][ T3416] Code: 66 89 01 31 c9 0f 1f 00 c3 cc cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 48 89 cb 48 c1 fb 3f 48 09 d9 0f 1f 00 <89> 01 31 c9 0 [ 80.476312][ T3416] RSP: 0018:ffffc90004dffa38 EFLAGS: 00010206 [ 80.476322][ T3416] RAX: 000000000000000c RBX: 0000000000000000 RCX: 0000200000000200 [ 80.476329][ T3416] RDX: 0000000000000000 RSI: ffff888016abe300 RDI: ffff888017878c20 [ 80.476335][ T3416] RBP: ffffc90004dffc10 R08: 0000000000000000 R09: 0000000000000000 [ 80.476340][ T3416] R10: 0000000000000000 R11: ffffffff82034b65 R12: ffff888017c0a1e8 [ 80.476346][ T3416] R13: ffff88800d6a8200 R14: 0000000000000000 R15: ffff888017c08a38 [ 80.476354][ T3416] ? __might_fault+0xb5/0x130 [ 80.476367][ T3416] copy_process+0x1f64/0x3d80 [ 80.476375][ T3416] ? lockdep_hardirqs_on_prepare+0x21/0x280 [ 80.476388][ T3416] ? copy_process+0x996/0x3d80 [ 80.476399][ T3416] ? __pfx_copy_process+0x10/0x10 [ 80.476406][ T3416] ? from_pool+0x1e1/0x750 [ 80.476416][ T3416] ? handle_mm_fault+0x122e/0x1ab0 [ 80.476432][ T3416] kernel_clone+0x216/0x940 [ 80.476440][ T3416] ? __pfx_llist_del_first+0x10/0x10 [ 80.476448][ T3416] ? check_new_class+0x28a/0xe90 [ 80.476458][ T3416] ? __pfx_kernel_clone+0x10/0x10 [ 80.476468][ T3416] ? from_pool+0x1e1/0x750 [ 80.476478][ T3416] ? __pfx_from_pool+0x10/0x10 [ 80.476487][ T3416] ? __pfx_from_pool+0x10/0x10 [ 80.476502][ T3416] __x64_sys_clone+0x18d/0x1f0 [ 80.476512][ T3416] ? __pfx___x64_sys_clone+0x10/0x10 [ 80.476520][ T3416] ? llist_add_batch+0x111/0x1f0 [ 80.476532][ T3416] ? dept_task+0x5/0x20 [ 80.476539][ T3416] ? dept_on+0x1c/0x30 [ 80.476545][ T3416] ? dept_exit+0x1c5/0x2c0 [ 80.476553][ T3416] ? lockdep_hardirqs_on_prepare+0x21/0x280 [ 80.476565][ T3416] do_syscall_64+0x6f/0x120 [ 80.476573][ T3416] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 80.476580][ T3416] RIP: 0033:0x41b26d [ 80.476588][ T3416] Code: b3 66 2e 0f 1f 84 00 00 00 00 00 66 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 8 [ 80.476595][ T3416] RSP: 002b:00007ffa1ad2d198 EFLAGS: 00000206 ORIG_RAX: 0000000000000038 [ 80.476604][ T3416] RAX: ffffffffffffffda RBX: 00007ffa1ad2dcdc RCX: 000000000041b26d [ 80.476610][ T3416] RDX: 0000200000000200 RSI: 0000000000000000 RDI: 0000000000001200 [ 80.476616][ T3416] RBP: 00007ffa1ad2d1e0 R08: 0000000000000000 R09: 0000000000000000 [ 80.476621][ T3416] R10: 0000000000000000 R11: 0000000000000206 R12: 00007ffa1ad2d6c0 [ 80.476626][ T3416] R13: ffffffffffffffb8 R14: 0000000000000002 R15: 00007ffd95d76940 [ 80.476638][ T3416] </TASK>
On Tue, May 13, 2025 at 05:34:48PM +0800, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log [3] in this scenario:
Process 1 Process 2
hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
...
Signed-off-by: Gavin Guo gavinguo@igalia.com
I think this is more convoluted that it needs to be?
hugetlb_wp() is called from hugetlb_no_page() and hugetlb_fault(). hugetlb_no_page() locks and unlocks the lock itself, which leaves us with hugetlb_fault().
hugetlb_fault() always passed the folio locked to hugetlb_wp(), and the latter only unlocks it when we have a cow from owner happening and we cannot satisfy the allocation. So, should not checking whether the folio is still locked after returning enough? What speaks against:
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index bd8971388236..23b57c5689a4 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6228,6 +6228,12 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash;
folio_put(old_folio); + /* + * The pagecache_folio needs to be unlocked to avoid + * deadlock when the child unmaps the folio. + */ + if (pagecache_folio) + folio_unlock(pagecache_folio); /* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock @@ -6825,7 +6831,12 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, spin_unlock(vmf.ptl);
if (pagecache_folio) { - folio_unlock(pagecache_folio); + /* + * hugetlb_wp() might have already unlocked pagecache_folio, so + * skip it if that is the case. + */ + if (folio_test_locked(pagecache_folio)) + folio_unlock(pagecache_folio); folio_put(pagecache_folio); } out_mutex:
mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
- Keep the pte_same checks anyway to make transition from the mutex easier.
*/ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio,
struct vm_fault *vmf)
struct vm_fault *vmf,bool *pagecache_folio_unlocked){ struct vm_area_struct *vma = vmf->vma; struct mm_struct *mm = vma->vm_mm; @@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio);
/** The pagecache_folio needs to be unlocked to avoid* deadlock and we won't re-lock it in hugetlb_wp(). The* pagecache_folio could be truncated after being* unlocked. So its state should not be relied* subsequently.** Setting *pagecache_folio_unlocked to true allows the* caller to handle any necessary logic related to the* folio's unlocked state.*/if (pagecache_folio) {folio_unlock(pagecache_folio);if (pagecache_folio_unlocked)*pagecache_folio_unlocked = true;} /* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock@@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */
ret = hugetlb_wp(folio, vmf);
ret = hugetlb_wp(folio, vmf, NULL);spin_unlock(vmf->ptl); @@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0;
- bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h),
@@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) {
ret = hugetlb_wp(pagecache_folio, &vmf);
ret = hugetlb_wp(pagecache_folio, &vmf,&pagecache_folio_unlocked); goto out_put_page;@@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl);
- if (pagecache_folio) {
- /*
* If the pagecache_folio is unlocked in hugetlb_wp(), we skip* folio_unlock() here.*/- if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio);
- if (pagecache_folio) folio_put(pagecache_folio);
- }
out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
2.43.0
On 5/21/25 03:53, Oscar Salvador wrote:
On Tue, May 13, 2025 at 05:34:48PM +0800, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log [3] in this scenario:
Process 1 Process 2
hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
...
Signed-off-by: Gavin Guo gavinguo@igalia.com
I think this is more convoluted that it needs to be?
hugetlb_wp() is called from hugetlb_no_page() and hugetlb_fault(). hugetlb_no_page() locks and unlocks the lock itself, which leaves us with hugetlb_fault().
hugetlb_fault() always passed the folio locked to hugetlb_wp(), and the latter only unlocks it when we have a cow from owner happening and we cannot satisfy the allocation. So, should not checking whether the folio is still locked after returning enough? What speaks against:
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index bd8971388236..23b57c5689a4 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6228,6 +6228,12 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio);
/** The pagecache_folio needs to be unlocked to avoid* deadlock when the child unmaps the folio.*/if (pagecache_folio)folio_unlock(pagecache_folio); /* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock@@ -6825,7 +6831,12 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, spin_unlock(vmf.ptl); if (pagecache_folio) {
folio_unlock(pagecache_folio);
/** hugetlb_wp() might have already unlocked pagecache_folio, so* skip it if that is the case.*/if (folio_test_locked(pagecache_folio))folio_unlock(pagecache_folio); folio_put(pagecache_folio);out_mutex:
Really appreciate your review. Good catch! This is elegant. The patch is under testing and I'll send out the v2 patch soon.
mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
- Keep the pte_same checks anyway to make transition from the mutex easier.
*/ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio,
struct vm_fault *vmf)
struct vm_fault *vmf,bool *pagecache_folio_unlocked)@@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio);
/** The pagecache_folio needs to be unlocked to avoid* deadlock and we won't re-lock it in hugetlb_wp(). The* pagecache_folio could be truncated after being* unlocked. So its state should not be relied* subsequently.** Setting *pagecache_folio_unlocked to true allows the* caller to handle any necessary logic related to the* folio's unlocked state.*/if (pagecache_folio) {folio_unlock(pagecache_folio);if (pagecache_folio_unlocked)*pagecache_folio_unlocked = true;} /* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock@@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */
ret = hugetlb_wp(folio, vmf);
ret = hugetlb_wp(folio, vmf, NULL);spin_unlock(vmf->ptl); @@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0;
- bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h),
@@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) {
ret = hugetlb_wp(pagecache_folio, &vmf);
ret = hugetlb_wp(pagecache_folio, &vmf,&pagecache_folio_unlocked); goto out_put_page;@@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl);
- if (pagecache_folio) {
- /*
* If the pagecache_folio is unlocked in hugetlb_wp(), we skip* folio_unlock() here.*/- if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio);
- if (pagecache_folio) folio_put(pagecache_folio);
- } out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
2.43.0
Hi Gavin,
On 5/13/25 7:34 PM, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log [3] in this scenario:
Process 1 Process 2
hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
This is a ABBA deadlock involving two locks:
- Lock A: pagecache_folio lock
- Lock B: hugetlb_fault_mutex_table lock
The deadlock occurs between two processes as follows:
- The first process (let’s call it Process 1) is handling a
copy-on-write (COW) operation on a hugepage via hugetlb_wp. Due to insufficient reserved hugetlb pages, Process 1, owner of the reserved hugetlb page, attempts to unmap a hugepage owned by another process (non-owner) to satisfy the reservation. Before unmapping, Process 1 acquires lock B (hugetlb_fault_mutex_table lock) and then lock A (pagecache_folio lock). To proceed with the unmap, it releases Lock B but retains Lock A. After the unmap, Process 1 tries to reacquire Lock B. However, at this point, Lock B has already been acquired by another process.
- The second process (Process 2) enters the hugetlb_fault handler
during the unmap operation. It successfully acquires Lock B (hugetlb_fault_mutex_table lock) that was just released by Process 1, but then attempts to acquire Lock A (pagecache_folio lock), which is still held by Process 1.
As a result, Process 1 (holding Lock A) is blocked waiting for Lock B (held by Process 2), while Process 2 (holding Lock B) is blocked waiting for Lock A (held by Process 1), constructing a ABBA deadlock scenario.
The solution here is to unlock the pagecache_folio and provide the pagecache_folio_unlocked variable to the caller to have the visibility over the pagecache_folio status for subsequent handling.
The error message: INFO: task repro_20250402_:13229 blocked for more than 64 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:25856 pid:13229 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00004006 Call Trace:
<TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 schedule_preempt_disabled+0x15/0x30 __mutex_lock+0x75f/0xeb0 hugetlb_wp+0xf88/0x3440 hugetlb_fault+0x14c8/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0x61d/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0010:__put_user_4+0xd/0x20 copy_process+0x1f4a/0x3d60 kernel_clone+0x210/0x8f0 __x64_sys_clone+0x18d/0x1f0 do_syscall_64+0x6a/0x120 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x41b26d </TASK> INFO: task repro_20250402_:13229 is blocked on a mutex likely owned by task repro_20250402_:13250. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK> INFO: task repro_20250402_:13250 blocked for more than 65 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK>
Showing all locks held in the system: 1 lock held by khungtaskd/35: #0: ffffffff879a7440 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x30/0x180 2 locks held by repro_20250402_/13229: #0: ffff888017d801e0 (&mm->mmap_lock){++++}-{4:4}, at: lock_mm_and_find_vma+0x37/0x300 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_wp+0xf88/0x3440 3 locks held by repro_20250402_/13250: #0: ffff8880177f3d08 (vm_lock){++++}-{0:0}, at: do_user_addr_fault+0x41b/0x1490 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_fault+0x3b8/0x2c30 #2: ffff8880129500e8 (&resv_map->rw_sema){++++}-{4:4}, at: hugetlb_fault+0x494/0x2c30
Link: https://drive.google.com/file/d/1DVRnIW-vSayU5J1re9Ct_br3jJQU6Vpb/view?usp=d... [1] Link: https://github.com/bboymimi/bpftracer/blob/master/scripts/hugetlb_lock_debug... [2] Link: https://drive.google.com/file/d/1bWq2-8o-BJAuhoHWX7zAhI6ggfhVzQUI/view?usp=s... [3] Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org Cc: Hugh Dickins hughd@google.com Cc: Florent Revest revest@google.com Cc: Gavin Shan gshan@redhat.com Signed-off-by: Gavin Guo gavinguo@igalia.com
mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
I guess the change log can become concise after the kernel log is dropped. The summarized stack trace is sufficient to indicate how the dead locking scenario happens. Besides, it's no need to mention bpftrace and its output. So the changelog would be simplified to something like below. Please polish it a bit if you would to take it. The solution looks good except some nitpicks as below.
---
There is ABBA dead locking scenario happening between hugetlb_fault() and hugetlb_wp() on the pagecache folio's lock and hugetlb global mutex, which is reproducible with syzkaller [1]. As below stack traces reveal, process-1 tries to take the hugetlb global mutex (A3), but with the pagecache folio's lock hold. Process-2 took the hugetlb global mutex but tries to take the pagecache folio's lock.
[1] https://drive.google.com/file/d/1DVRnIW-vSayU5J1re9Ct_br3jJQU6Vpb/view?usp=d...
Process-1 Process-2 ========= ========= hugetlb_fault mutex_lock (A1) filemap_lock_hugetlb_folio (B1) hugetlb_wp alloc_hugetlb_folio #error mutex_unlock (A2) hugetlb_fault mutex_lock (A4) filemap_lock_hugetlb_folio (B4) unmap_ref_private mutex_lock (A3)
Fix it by releasing the pagecache folio's lock at (A2) of process-1 so that pagecache folio's lock is available to process-2 at (B4), to avoid the deadlock. In process-1, a new variable is added to track if the pagecache folio's lock has been released by its child function hugetlb_wp() to avoid double releases on the lock in hugetlb_fault(). The similar changes are applied to hugetlb_no_page().
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
- Keep the pte_same checks anyway to make transition from the mutex easier.
*/ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio,
struct vm_fault *vmf)
struct vm_fault *vmf,bool *pagecache_folio_unlocked)
Nitpick: the variable may be renamed to 'pagecache_folio_locked' if you're happy with.
{ struct vm_area_struct *vma = vmf->vma; struct mm_struct *mm = vma->vm_mm; @@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio);
/** The pagecache_folio needs to be unlocked to avoid
^^^^^^^^
has to be (?)
* deadlock and we won't re-lock it in hugetlb_wp(). The* pagecache_folio could be truncated after being* unlocked. So its state should not be relied
^^^^^^ reliable (?)
* subsequently.** Setting *pagecache_folio_unlocked to true allows the* caller to handle any necessary logic related to the* folio's unlocked state.*/if (pagecache_folio) {folio_unlock(pagecache_folio);if (pagecache_folio_unlocked)*pagecache_folio_unlocked = true;}
The second section of the comments looks a bit redundant since the code changes are self-explaining enough :-)
/* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock@@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */
ret = hugetlb_wp(folio, vmf);
ret = hugetlb_wp(folio, vmf, NULL);
It's not certain if we have another deadlock between hugetlb_no_page() and hugetlb_wp(), similar to the existing one between hugetlb_fault() and hugetlb_wp(). So I think it's reasonable to pass '&pagecache_folio_locked' to hugetlb_wp() here and skip to unlock on pagecache_folio_locked == false in hugetlb_no_page(). It's not harmful at least.
} spin_unlock(vmf->ptl); @@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0;
- bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h),
@@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) {
ret = hugetlb_wp(pagecache_folio, &vmf);
ret = hugetlb_wp(pagecache_folio, &vmf,&pagecache_folio_unlocked); goto out_put_page;@@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl);
- if (pagecache_folio) {
- /*
* If the pagecache_folio is unlocked in hugetlb_wp(), we skip* folio_unlock() here.*/- if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio);
- if (pagecache_folio) folio_put(pagecache_folio);
- }
The comments seem redundant since the code changes are self-explaining. Besides, no need to validate 'pagecache_folio' for twice.
if (pagecache_folio) { if (pagecache_folio_locked) folio_unlock(pagecache_folio);
folio_put(pagecache_folio); }
out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
Thanks, Gavin
Hi Gavin,
On 5/26/25 12:41, Gavin Shan wrote:
Hi Gavin,
On 5/13/25 7:34 PM, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log [3] in this scenario:
Process 1 Process 2 --- --- hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
This is a ABBA deadlock involving two locks:
- Lock A: pagecache_folio lock
- Lock B: hugetlb_fault_mutex_table lock
The deadlock occurs between two processes as follows:
- The first process (let’s call it Process 1) is handling a
copy-on-write (COW) operation on a hugepage via hugetlb_wp. Due to insufficient reserved hugetlb pages, Process 1, owner of the reserved hugetlb page, attempts to unmap a hugepage owned by another process (non-owner) to satisfy the reservation. Before unmapping, Process 1 acquires lock B (hugetlb_fault_mutex_table lock) and then lock A (pagecache_folio lock). To proceed with the unmap, it releases Lock B but retains Lock A. After the unmap, Process 1 tries to reacquire Lock B. However, at this point, Lock B has already been acquired by another process.
- The second process (Process 2) enters the hugetlb_fault handler
during the unmap operation. It successfully acquires Lock B (hugetlb_fault_mutex_table lock) that was just released by Process 1, but then attempts to acquire Lock A (pagecache_folio lock), which is still held by Process 1.
As a result, Process 1 (holding Lock A) is blocked waiting for Lock B (held by Process 2), while Process 2 (holding Lock B) is blocked waiting for Lock A (held by Process 1), constructing a ABBA deadlock scenario.
The solution here is to unlock the pagecache_folio and provide the pagecache_folio_unlocked variable to the caller to have the visibility over the pagecache_folio status for subsequent handling.
The error message: INFO: task repro_20250402_:13229 blocked for more than 64 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:25856 pid:13229 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00004006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 schedule_preempt_disabled+0x15/0x30 __mutex_lock+0x75f/0xeb0 hugetlb_wp+0xf88/0x3440 hugetlb_fault+0x14c8/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0x61d/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0010:__put_user_4+0xd/0x20 copy_process+0x1f4a/0x3d60 kernel_clone+0x210/0x8f0 __x64_sys_clone+0x18d/0x1f0 do_syscall_64+0x6a/0x120 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x41b26d </TASK> INFO: task repro_20250402_:13229 is blocked on a mutex likely owned by task repro_20250402_:13250. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK> INFO: task repro_20250402_:13250 blocked for more than 65 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK>
Showing all locks held in the system: 1 lock held by khungtaskd/35: #0: ffffffff879a7440 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x30/0x180 2 locks held by repro_20250402_/13229: #0: ffff888017d801e0 (&mm->mmap_lock){++++}-{4:4}, at: lock_mm_and_find_vma+0x37/0x300 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_wp+0xf88/0x3440 3 locks held by repro_20250402_/13250: #0: ffff8880177f3d08 (vm_lock){++++}-{0:0}, at: do_user_addr_fault+0x41b/0x1490 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_fault+0x3b8/0x2c30 #2: ffff8880129500e8 (&resv_map->rw_sema){++++}-{4:4}, at: hugetlb_fault+0x494/0x2c30
Link: https://drive.google.com/file/d/1DVRnIW- vSayU5J1re9Ct_br3jJQU6Vpb/view?usp=drive_link [1] Link: https://github.com/bboymimi/bpftracer/blob/master/scripts/ hugetlb_lock_debug.bt [2] Link: https://drive.google.com/file/d/1bWq2-8o- BJAuhoHWX7zAhI6ggfhVzQUI/view?usp=sharing [3] Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org Cc: Hugh Dickins hughd@google.com Cc: Florent Revest revest@google.com Cc: Gavin Shan gshan@redhat.com Signed-off-by: Gavin Guo gavinguo@igalia.com
mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
I guess the change log can become concise after the kernel log is dropped. The summarized stack trace is sufficient to indicate how the dead locking scenario happens. Besides, it's no need to mention bpftrace and its output. So the changelog would be simplified to something like below. Please polish it a bit if you would to take it. The solution looks good except some nitpicks as below.
There is ABBA dead locking scenario happening between hugetlb_fault() and hugetlb_wp() on the pagecache folio's lock and hugetlb global mutex, which is reproducible with syzkaller [1]. As below stack traces reveal, process-1 tries to take the hugetlb global mutex (A3), but with the pagecache folio's lock hold. Process-2 took the hugetlb global mutex but tries to take the pagecache folio's lock.
[1] https://drive.google.com/file/d/1DVRnIW-vSayU5J1re9Ct_br3jJQU6Vpb/ view?usp=drive_link
Process-1 Process-2 ========= ========= hugetlb_fault mutex_lock (A1) filemap_lock_hugetlb_folio (B1) hugetlb_wp alloc_hugetlb_folio #error mutex_unlock (A2) hugetlb_fault mutex_lock (A4) filemap_lock_hugetlb_folio (B4) unmap_ref_private mutex_lock (A3)
Fix it by releasing the pagecache folio's lock at (A2) of process-1 so that pagecache folio's lock is available to process-2 at (B4), to avoid the deadlock. In process-1, a new variable is added to track if the pagecache folio's lock has been released by its child function hugetlb_wp() to avoid double releases on the lock in hugetlb_fault(). The similar changes are applied to hugetlb_no_page().
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma, * Keep the pte_same checks anyway to make transition from the mutex easier. */ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, - struct vm_fault *vmf) + struct vm_fault *vmf, + bool *pagecache_folio_unlocked)
Nitpick: the variable may be renamed to 'pagecache_folio_locked' if you're happy with.
{ struct vm_area_struct *vma = vmf->vma; struct mm_struct *mm = vma->vm_mm; @@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio); + /* + * The pagecache_folio needs to be unlocked to avoid
^^^^^^^^
has to be (?)
+ * deadlock and we won't re-lock it in hugetlb_wp(). The + * pagecache_folio could be truncated after being + * unlocked. So its state should not be relied
^^^^^^ reliable (?)
+ * subsequently. + * + * Setting *pagecache_folio_unlocked to true allows the + * caller to handle any necessary logic related to the + * folio's unlocked state. + */ + if (pagecache_folio) { + folio_unlock(pagecache_folio); + if (pagecache_folio_unlocked) + *pagecache_folio_unlocked = true; + }
The second section of the comments looks a bit redundant since the code changes are self-explaining enough :-)
/* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock @@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */ - ret = hugetlb_wp(folio, vmf); + ret = hugetlb_wp(folio, vmf, NULL);
It's not certain if we have another deadlock between hugetlb_no_page() and hugetlb_wp(), similar to the existing one between hugetlb_fault() and hugetlb_wp(). So I think it's reasonable to pass '&pagecache_folio_locked' to hugetlb_wp() here and skip to unlock on pagecache_folio_locked == false in hugetlb_no_page(). It's not harmful at least.
Thank you very much for taking the time to review my patch! I appreciate your feedback. :)
After carefully reviewing the hugetlb_no_page function, I've made an observation regarding the pagecache_folio handling. Specifically, when pagecache_folio is assigned to vmf->pte, the vmf->ptl lock is held. This lock remains active when vmf->pte is later accessed in hugetlb_wp. The ptl lock ensures that the pte value is the same as the pagecache_folio assigned in hugetlb_no_page. As a result, the following comparison in hugetlb_wp will always evaluate to false because old_folio and pagecache_folio reference the same object:
if (is_vma_resv_set(vma, HPAGE_RESV_OWNER) && old_folio != pagecache_folio) cow_from_owner = true;
Based on this analysis, passing pagecache_folio_locked in hugetlb_no_page is unnecessary. Let me know if I missed anything. Other comments look good to me. Thanks!
} spin_unlock(vmf->ptl); @@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0; + bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h), @@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) { - ret = hugetlb_wp(pagecache_folio, &vmf); + ret = hugetlb_wp(pagecache_folio, &vmf, + &pagecache_folio_unlocked); goto out_put_page; } else if (likely(flags & FAULT_FLAG_WRITE)) { vmf.orig_pte = huge_pte_mkdirty(vmf.orig_pte); @@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl); - if (pagecache_folio) { + /* + * If the pagecache_folio is unlocked in hugetlb_wp(), we skip + * folio_unlock() here. + */ + if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio); + if (pagecache_folio) folio_put(pagecache_folio); - }
The comments seem redundant since the code changes are self-explaining. Besides, no need to validate 'pagecache_folio' for twice.
if (pagecache_folio) { if (pagecache_folio_locked) folio_unlock(pagecache_folio);
folio_put(pagecache_folio); }
out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
Thanks, Gavin
Hi Gavin,
On 5/27/25 7:59 PM, Gavin Guo wrote:
On 5/26/25 12:41, Gavin Shan wrote:
On 5/13/25 7:34 PM, Gavin Guo wrote:
The patch fixes a deadlock which can be triggered by an internal syzkaller [1] reproducer and captured by bpftrace script [2] and its log [3] in this scenario:
Process 1 Process 2 --- --- hugetlb_fault mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // take A hugetlb_wp mutex_unlock(B) // release B ... hugetlb_fault ... mutex_lock(B) // take B filemap_lock_hugetlb_folio filemap_lock_folio __filemap_get_folio folio_lock(A) // blocked unmap_ref_private ... mutex_lock(B) // retake and blocked
This is a ABBA deadlock involving two locks:
- Lock A: pagecache_folio lock
- Lock B: hugetlb_fault_mutex_table lock
The deadlock occurs between two processes as follows:
- The first process (let’s call it Process 1) is handling a
copy-on-write (COW) operation on a hugepage via hugetlb_wp. Due to insufficient reserved hugetlb pages, Process 1, owner of the reserved hugetlb page, attempts to unmap a hugepage owned by another process (non-owner) to satisfy the reservation. Before unmapping, Process 1 acquires lock B (hugetlb_fault_mutex_table lock) and then lock A (pagecache_folio lock). To proceed with the unmap, it releases Lock B but retains Lock A. After the unmap, Process 1 tries to reacquire Lock B. However, at this point, Lock B has already been acquired by another process.
- The second process (Process 2) enters the hugetlb_fault handler
during the unmap operation. It successfully acquires Lock B (hugetlb_fault_mutex_table lock) that was just released by Process 1, but then attempts to acquire Lock A (pagecache_folio lock), which is still held by Process 1.
As a result, Process 1 (holding Lock A) is blocked waiting for Lock B (held by Process 2), while Process 2 (holding Lock B) is blocked waiting for Lock A (held by Process 1), constructing a ABBA deadlock scenario.
The solution here is to unlock the pagecache_folio and provide the pagecache_folio_unlocked variable to the caller to have the visibility over the pagecache_folio status for subsequent handling.
The error message: INFO: task repro_20250402_:13229 blocked for more than 64 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:25856 pid:13229 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00004006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 schedule_preempt_disabled+0x15/0x30 __mutex_lock+0x75f/0xeb0 hugetlb_wp+0xf88/0x3440 hugetlb_fault+0x14c8/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0x61d/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0010:__put_user_4+0xd/0x20 copy_process+0x1f4a/0x3d60 kernel_clone+0x210/0x8f0 __x64_sys_clone+0x18d/0x1f0 do_syscall_64+0x6a/0x120 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x41b26d </TASK> INFO: task repro_20250402_:13229 is blocked on a mutex likely owned by task repro_20250402_:13250. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK> INFO: task repro_20250402_:13250 blocked for more than 65 seconds. Not tainted 6.15.0-rc3+ #24 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:repro_20250402_ state:D stack:28288 pid:13250 tgid:13228 ppid:3513 task_flags:0x400040 flags:0x00000006 Call Trace: <TASK> __schedule+0x1755/0x4f50 schedule+0x158/0x330 io_schedule+0x92/0x110 folio_wait_bit_common+0x69a/0xba0 __filemap_get_folio+0x154/0xb70 hugetlb_fault+0xa50/0x2c30 trace_clock_x86_tsc+0x20/0x20 do_user_addr_fault+0xace/0x1490 exc_page_fault+0x64/0x100 asm_exc_page_fault+0x26/0x30 RIP: 0033:0x402619 </TASK>
Showing all locks held in the system: 1 lock held by khungtaskd/35: #0: ffffffff879a7440 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x30/0x180 2 locks held by repro_20250402_/13229: #0: ffff888017d801e0 (&mm->mmap_lock){++++}-{4:4}, at: lock_mm_and_find_vma+0x37/0x300 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_wp+0xf88/0x3440 3 locks held by repro_20250402_/13250: #0: ffff8880177f3d08 (vm_lock){++++}-{0:0}, at: do_user_addr_fault+0x41b/0x1490 #1: ffff888000fec848 (&hugetlb_fault_mutex_table[i]){+.+.}-{4:4}, at: hugetlb_fault+0x3b8/0x2c30 #2: ffff8880129500e8 (&resv_map->rw_sema){++++}-{4:4}, at: hugetlb_fault+0x494/0x2c30
Link: https://drive.google.com/file/d/1DVRnIW- vSayU5J1re9Ct_br3jJQU6Vpb/view?usp=drive_link [1] Link: https://github.com/bboymimi/bpftracer/blob/master/scripts/ hugetlb_lock_debug.bt [2] Link: https://drive.google.com/file/d/1bWq2-8o- BJAuhoHWX7zAhI6ggfhVzQUI/view?usp=sharing [3] Fixes: 40549ba8f8e0 ("hugetlb: use new vma_lock for pmd sharing synchronization") Cc: stable@vger.kernel.org Cc: Hugh Dickins hughd@google.com Cc: Florent Revest revest@google.com Cc: Gavin Shan gshan@redhat.com Signed-off-by: Gavin Guo gavinguo@igalia.com
mm/hugetlb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
I guess the change log can become concise after the kernel log is dropped. The summarized stack trace is sufficient to indicate how the dead locking scenario happens. Besides, it's no need to mention bpftrace and its output. So the changelog would be simplified to something like below. Please polish it a bit if you would to take it. The solution looks good except some nitpicks as below.
There is ABBA dead locking scenario happening between hugetlb_fault() and hugetlb_wp() on the pagecache folio's lock and hugetlb global mutex, which is reproducible with syzkaller [1]. As below stack traces reveal, process-1 tries to take the hugetlb global mutex (A3), but with the pagecache folio's lock hold. Process-2 took the hugetlb global mutex but tries to take the pagecache folio's lock.
[1] https://drive.google.com/file/d/1DVRnIW-vSayU5J1re9Ct_br3jJQU6Vpb/ view?usp=drive_link
Process-1 Process-2 ========= ========= hugetlb_fault mutex_lock (A1) filemap_lock_hugetlb_folio (B1) hugetlb_wp alloc_hugetlb_folio #error mutex_unlock (A2) hugetlb_fault mutex_lock (A4) filemap_lock_hugetlb_folio (B4) unmap_ref_private mutex_lock (A3)
Fix it by releasing the pagecache folio's lock at (A2) of process-1 so that pagecache folio's lock is available to process-2 at (B4), to avoid the deadlock. In process-1, a new variable is added to track if the pagecache folio's lock has been released by its child function hugetlb_wp() to avoid double releases on the lock in hugetlb_fault(). The similar changes are applied to hugetlb_no_page().
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3e6ac991b9c..ad54a74aa563 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6115,7 +6115,8 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma, * Keep the pte_same checks anyway to make transition from the mutex easier. */ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, - struct vm_fault *vmf) + struct vm_fault *vmf, + bool *pagecache_folio_unlocked)
Nitpick: the variable may be renamed to 'pagecache_folio_locked' if you're happy with.
{ struct vm_area_struct *vma = vmf->vma; struct mm_struct *mm = vma->vm_mm; @@ -6212,6 +6213,22 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio); + /* + * The pagecache_folio needs to be unlocked to avoid
^^^^^^^^
has to be (?)
+ * deadlock and we won't re-lock it in hugetlb_wp(). The + * pagecache_folio could be truncated after being + * unlocked. So its state should not be relied
^^^^^^ reliable (?)
+ * subsequently. + * + * Setting *pagecache_folio_unlocked to true allows the + * caller to handle any necessary logic related to the + * folio's unlocked state. + */ + if (pagecache_folio) { + folio_unlock(pagecache_folio); + if (pagecache_folio_unlocked) + *pagecache_folio_unlocked = true; + }
The second section of the comments looks a bit redundant since the code changes are self-explaining enough :-)
/* * Drop hugetlb_fault_mutex and vma_lock before * unmapping. unmapping needs to hold vma_lock @@ -6566,7 +6583,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping, hugetlb_count_add(pages_per_huge_page(h), mm); if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) { /* Optimization, do the COW without a second fault */ - ret = hugetlb_wp(folio, vmf); + ret = hugetlb_wp(folio, vmf, NULL);
It's not certain if we have another deadlock between hugetlb_no_page() and hugetlb_wp(), similar to the existing one between hugetlb_fault() and hugetlb_wp(). So I think it's reasonable to pass '&pagecache_folio_locked' to hugetlb_wp() here and skip to unlock on pagecache_folio_locked == false in hugetlb_no_page(). It's not harmful at least.
Thank you very much for taking the time to review my patch! I appreciate your feedback. :)
After carefully reviewing the hugetlb_no_page function, I've made an observation regarding the pagecache_folio handling. Specifically, when pagecache_folio is assigned to vmf->pte, the vmf->ptl lock is held. This lock remains active when vmf->pte is later accessed in hugetlb_wp. The ptl lock ensures that the pte value is the same as the pagecache_folio assigned in hugetlb_no_page. As a result, the following comparison in hugetlb_wp will always evaluate to false because old_folio and pagecache_folio reference the same object:
if (is_vma_resv_set(vma, HPAGE_RESV_OWNER) && old_folio != pagecache_folio) cow_from_owner = true;
Based on this analysis, passing pagecache_folio_locked in hugetlb_no_page is unnecessary. Let me know if I missed anything. Other comments look good to me. Thanks!
Thanks for diving into the code deeper. Agreed, we're safe to bypass this specific case. As you explained, @old_folio sorted out from PTE should be equal to @pagecache_folio, and the consistence is guranteed by PTL lock. So we don't have locking contentions between hugetlb_no_page() and hugetlb_wp().
} spin_unlock(vmf->ptl); @@ -6638,6 +6655,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, struct hstate *h = hstate_vma(vma); struct address_space *mapping; int need_wait_lock = 0; + bool pagecache_folio_unlocked = false; struct vm_fault vmf = { .vma = vma, .address = address & huge_page_mask(h), @@ -6792,7 +6810,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(vmf.orig_pte)) { - ret = hugetlb_wp(pagecache_folio, &vmf); + ret = hugetlb_wp(pagecache_folio, &vmf, + &pagecache_folio_unlocked); goto out_put_page; } else if (likely(flags & FAULT_FLAG_WRITE)) { vmf.orig_pte = huge_pte_mkdirty(vmf.orig_pte); @@ -6809,10 +6828,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl); - if (pagecache_folio) { + /* + * If the pagecache_folio is unlocked in hugetlb_wp(), we skip + * folio_unlock() here. + */ + if (pagecache_folio && !pagecache_folio_unlocked) folio_unlock(pagecache_folio); + if (pagecache_folio) folio_put(pagecache_folio); - }
The comments seem redundant since the code changes are self-explaining. Besides, no need to validate 'pagecache_folio' for twice.
if (pagecache_folio) { if (pagecache_folio_locked) folio_unlock(pagecache_folio);
folio_put(pagecache_folio); }
out_mutex: hugetlb_vma_unlock_read(vma);
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
Thanks, Gavin
linux-stable-mirror@lists.linaro.org
-
Andrew Morton -
Byungchul Park -
Gavin Guo -
Gavin Shan -
Oscar Salvador