* [PATCH v2] drm/shmem-helper: Fix Map huge page mapping in fault handler
@ 2026-03-13 14:17 Pedro Demarchi Gomes
2026-03-13 17:08 ` Boris Brezillon
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Pedro Demarchi Gomes @ 2026-03-13 14:17 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Boris Brezillon, Loic Molinari
Cc: dri-devel, linux-kernel, Pedro Demarchi Gomes
When running ./tools/testing/selftests/mm/split_huge_page_test multiple
times with /sys/kernel/mm/transparent_hugepage/shmem_enabled and
/sys/kernel/mm/transparent_hugepage/enabled set as always the following BUG
occurs:
[ 232.728858] ------------[ cut here ]------------
[ 232.729458] kernel BUG at mm/memory.c:2276!
[ 232.729726] Oops: invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 232.730217] CPU: 19 UID: 60578 PID: 1497 Comm: llvmpipe-9 Not tainted 7.0.0-rc1mm-new+ #19 PREEMPT(lazy)
[ 232.730855] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-9.fc43 06/10/2025
[ 232.731360] RIP: 0010:walk_to_pmd+0x29e/0x3c0
[ 232.731569] Code: d8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 48 89 ea 48 89 de 4c 89 f7 e8 ae 85 ff ff 85 c0 0f 84 1f fe ff ff 31 db eb d0 <0f> 0b 48 89 ea 48 89 de 4c 89 f7 e8 92 8b ff ff 85 c0 75 e8 48 b8
[ 232.732614] RSP: 0000:ffff8881aa6ff9a8 EFLAGS: 00010282
[ 232.732991] RAX: 8000000142e002e7 RBX: ffff8881433cae10 RCX: dffffc0000000000
[ 232.733362] RDX: 0000000000000000 RSI: 00007fb47840b000 RDI: 8000000142e002e7
[ 232.733801] RBP: 00007fb47840b000 R08: 0000000000000000 R09: 1ffff110354dff46
[ 232.734168] R10: fffffbfff0cb921d R11: 00000000910da5ce R12: 1ffffffff0c1fcdd
[ 232.734459] R13: 1ffffffff0c23f36 R14: ffff888171628040 R15: 0000000000000000
[ 232.734861] FS: 00007fb4907f86c0(0000) GS:ffff888791f2c000(0000) knlGS:0000000000000000
[ 232.735265] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 232.735548] CR2: 00007fb47840be00 CR3: 000000015e6dc000 CR4: 00000000000006f0
[ 232.736031] Call Trace:
[ 232.736273] <TASK>
[ 232.736500] get_locked_pte+0x1f/0xa0
[ 232.736878] insert_pfn+0x9f/0x350
[ 232.737190] ? __pfx_pat_pagerange_is_ram+0x10/0x10
[ 232.737614] ? __pfx_insert_pfn+0x10/0x10
[ 232.737990] ? __pfx_css_rstat_updated+0x10/0x10
[ 232.738281] ? __pfx_pfn_modify_allowed+0x10/0x10
[ 232.738552] ? lookup_memtype+0x62/0x180
[ 232.738761] vmf_insert_pfn_prot+0x14b/0x340
[ 232.739012] ? __pfx_vmf_insert_pfn_prot+0x10/0x10
[ 232.739247] ? __pfx___might_resched+0x10/0x10
[ 232.739475] drm_gem_shmem_fault.cold+0x18/0x39
[ 232.739677] ? rcu_read_unlock+0x20/0x70
[ 232.739882] __do_fault+0x251/0x7b0
[ 232.740028] do_fault+0x6e1/0xc00
[ 232.740167] ? __lock_acquire+0x590/0xc40
[ 232.740335] handle_pte_fault+0x439/0x760
[ 232.740498] ? mtree_range_walk+0x252/0xae0
[ 232.740669] ? __pfx_handle_pte_fault+0x10/0x10
[ 232.740899] __handle_mm_fault+0xa02/0xf30
[ 232.741066] ? __pfx___handle_mm_fault+0x10/0x10
[ 232.741255] ? find_vma+0xa1/0x120
[ 232.741403] handle_mm_fault+0x2bf/0x8f0
[ 232.741564] do_user_addr_fault+0x2d3/0xed0
[ 232.741736] ? trace_page_fault_user+0x1bf/0x240
[ 232.741969] exc_page_fault+0x87/0x120
[ 232.742124] asm_exc_page_fault+0x26/0x30
[ 232.742288] RIP: 0033:0x7fb4d73ed546
[ 232.742441] Code: 66 41 0f 6f fb 66 44 0f 6d dc 66 44 0f 6f c6 66 41 0f 6d f1 66 0f 6c fc 66 45 0f 6c c1 66 44 0f 6f c9 66 0f 6d ca 66 0f db f0 <66> 0f df 04 08 66 44 0f 6c ca 66 45 0f db c2 66 44 0f df 10 66 44
[ 232.743193] RSP: 002b:00007fb4907f68a0 EFLAGS: 00010206
[ 232.743565] RAX: 00007fb47840aa00 RBX: 00007fb4d73ec070 RCX: 0000000000001400
[ 232.743871] RDX: 0000000000002800 RSI: 0000000000003c00 RDI: 0000000000000001
[ 232.744150] RBP: 0000000000000004 R08: 0000000000001400 R09: 00007fb4d73ec060
[ 232.744433] R10: 000055f0261a4288 R11: 00007fb4c013da40 R12: 0000000000000008
[ 232.744712] R13: 0000000000000000 R14: 4332322132212110 R15: 0000000000000004
[ 232.746616] </TASK>
[ 232.746711] Modules linked in: nft_nat nft_masq veth bridge stp llc snd_seq_dummy snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore overlay rfkill nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables qrtr ppdev 9pnet_virtio 9pnet parport_pc i2c_piix4 netfs pcspkr parport i2c_smbus joydev sunrpc vfat fat loop dm_multipath nfnetlink vsock_loopback vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport zram lz4hc_compress vmw_vmci lz4_compress vsock e1000 bochs serio_raw ata_generic pata_acpi scsi_dh_rdac scsi_dh_emc scsi_dh_alua i2c_dev fuse qemu_fw_cfg
[ 232.749308] ---[ end trace 0000000000000000 ]---
[ 232.749507] RIP: 0010:walk_to_pmd+0x29e/0x3c0
[ 232.749692] Code: d8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 48 89 ea 48 89 de 4c 89 f7 e8 ae 85 ff ff 85 c0 0f 84 1f fe ff ff 31 db eb d0 <0f> 0b 48 89 ea 48 89 de 4c 89 f7 e8 92 8b ff ff 85 c0 75 e8 48 b8
[ 232.750428] RSP: 0000:ffff8881aa6ff9a8 EFLAGS: 00010282
[ 232.750645] RAX: 8000000142e002e7 RBX: ffff8881433cae10 RCX: dffffc0000000000
[ 232.750954] RDX: 0000000000000000 RSI: 00007fb47840b000 RDI: 8000000142e002e7
[ 232.751232] RBP: 00007fb47840b000 R08: 0000000000000000 R09: 1ffff110354dff46
[ 232.751514] R10: fffffbfff0cb921d R11: 00000000910da5ce R12: 1ffffffff0c1fcdd
[ 232.751837] R13: 1ffffffff0c23f36 R14: ffff888171628040 R15: 0000000000000000
[ 232.752124] FS: 00007fb4907f86c0(0000) GS:ffff888791f2c000(0000) knlGS:0000000000000000
[ 232.752441] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 232.752674] CR2: 00007fb47840be00 CR3: 000000015e6dc000 CR4: 00000000000006f0
[ 232.752983] Kernel panic - not syncing: Fatal exception
[ 232.753510] Kernel Offset: disabled
[ 232.754643] ---[ end Kernel panic - not syncing: Fatal exception ]---
This happens when two concurrent page faults occur within the same PMD range.
One fault installs a PMD mapping through vmf_insert_pfn_pmd(), while the other
attempts to install a PTE mapping via vmf_insert_pfn(). The bug is
triggered because a pmd_trans_huge is not expected when walking the page
table inside vmf_insert_pfn.
Avoid this race by adding a huge_fault callback to drm_gem_shmem_vm_ops so that
PMD-sized mappings are handled through the appropriate huge page fault path.
Fixes: 211b9a39f261 ("drm/shmem-helper: Map huge pages in fault handler")
Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
---
Changes in v2:
- Keep the #ifdef unindented
- Create drm_gem_shmem_any_fault to handle faults of any order and use
drm_gem_shmem_[huge_]fault() as wrappers
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 67 +++++++++++++++-----------
1 file changed, 39 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 7b5a49935ae4..fb8b74ac3057 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -550,36 +550,19 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
-static bool drm_gem_shmem_try_map_pmd(struct vm_fault *vmf, unsigned long addr,
- struct page *page)
-{
-#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
- unsigned long pfn = page_to_pfn(page);
- unsigned long paddr = pfn << PAGE_SHIFT;
- bool aligned = (addr & ~PMD_MASK) == (paddr & ~PMD_MASK);
-
- if (aligned &&
- pmd_none(*vmf->pmd) &&
- folio_test_pmd_mappable(page_folio(page))) {
- pfn &= PMD_MASK >> PAGE_SHIFT;
- if (vmf_insert_pfn_pmd(vmf, pfn, false) == VM_FAULT_NOPAGE)
- return true;
- }
-#endif
-
- return false;
-}
-
-static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
+static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf,
+ unsigned int order)
{
struct vm_area_struct *vma = vmf->vma;
struct drm_gem_object *obj = vma->vm_private_data;
struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
loff_t num_pages = obj->size >> PAGE_SHIFT;
- vm_fault_t ret;
+ vm_fault_t ret = VM_FAULT_FALLBACK;
struct page **pages = shmem->pages;
pgoff_t page_offset;
unsigned long pfn;
+ unsigned long paddr;
+ bool aligned;
/* Offset to faulty address in the VMA. */
page_offset = vmf->pgoff - vma->vm_pgoff;
@@ -593,13 +576,24 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
goto out;
}
- if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, pages[page_offset])) {
- ret = VM_FAULT_NOPAGE;
- goto out;
- }
-
pfn = page_to_pfn(pages[page_offset]);
- ret = vmf_insert_pfn(vma, vmf->address, pfn);
+ switch (order) {
+ case 0:
+ ret = vmf_insert_pfn(vma, vmf->address, pfn);
+ break;
+#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
+ case PMD_ORDER:
+ paddr = pfn << PAGE_SHIFT;
+ aligned = (vmf->address & ~PMD_MASK) == (paddr & ~PMD_MASK);
+
+ if (aligned &&
+ folio_test_pmd_mappable(page_folio(pages[page_offset]))) {
+ pfn &= PMD_MASK >> PAGE_SHIFT;
+ ret = vmf_insert_pfn_pmd(vmf, pfn, false);
+ }
+ break;
+#endif
+ }
out:
dma_resv_unlock(shmem->base.resv);
@@ -607,6 +601,20 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
return ret;
}
+static vm_fault_t drm_gem_shmem_huge_fault(struct vm_fault *vmf,
+ unsigned int order)
+{
+ if (order != PMD_ORDER)
+ return VM_FAULT_FALLBACK;
+
+ return drm_gem_shmem_any_fault(vmf, order);
+}
+
+static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
+{
+ return drm_gem_shmem_any_fault(vmf, 0);
+}
+
static void drm_gem_shmem_vm_open(struct vm_area_struct *vma)
{
struct drm_gem_object *obj = vma->vm_private_data;
@@ -643,6 +651,9 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
const struct vm_operations_struct drm_gem_shmem_vm_ops = {
.fault = drm_gem_shmem_fault,
+#ifdef CONFIG_ARCH_SUPPORTS_HUGE_PFNMAP
+ .huge_fault = drm_gem_shmem_huge_fault,
+#endif
.open = drm_gem_shmem_vm_open,
.close = drm_gem_shmem_vm_close,
};
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/shmem-helper: Fix Map huge page mapping in fault handler
2026-03-13 14:17 [PATCH v2] drm/shmem-helper: Fix Map huge page mapping in fault handler Pedro Demarchi Gomes
@ 2026-03-13 17:08 ` Boris Brezillon
2026-03-13 20:52 ` Claude review: " Claude Code Review Bot
2026-03-13 20:52 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2026-03-13 17:08 UTC (permalink / raw)
To: Pedro Demarchi Gomes
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Loic Molinari, dri-devel, linux-kernel
On Fri, 13 Mar 2026 11:17:19 -0300
Pedro Demarchi Gomes <pedrodemargomes@gmail.com> wrote:
> When running ./tools/testing/selftests/mm/split_huge_page_test multiple
> times with /sys/kernel/mm/transparent_hugepage/shmem_enabled and
> /sys/kernel/mm/transparent_hugepage/enabled set as always the following BUG
> occurs:
>
> [ 232.728858] ------------[ cut here ]------------
> [ 232.729458] kernel BUG at mm/memory.c:2276!
> [ 232.729726] Oops: invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
> [ 232.730217] CPU: 19 UID: 60578 PID: 1497 Comm: llvmpipe-9 Not tainted 7.0.0-rc1mm-new+ #19 PREEMPT(lazy)
> [ 232.730855] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-9.fc43 06/10/2025
> [ 232.731360] RIP: 0010:walk_to_pmd+0x29e/0x3c0
> [ 232.731569] Code: d8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 48 89 ea 48 89 de 4c 89 f7 e8 ae 85 ff ff 85 c0 0f 84 1f fe ff ff 31 db eb d0 <0f> 0b 48 89 ea 48 89 de 4c 89 f7 e8 92 8b ff ff 85 c0 75 e8 48 b8
> [ 232.732614] RSP: 0000:ffff8881aa6ff9a8 EFLAGS: 00010282
> [ 232.732991] RAX: 8000000142e002e7 RBX: ffff8881433cae10 RCX: dffffc0000000000
> [ 232.733362] RDX: 0000000000000000 RSI: 00007fb47840b000 RDI: 8000000142e002e7
> [ 232.733801] RBP: 00007fb47840b000 R08: 0000000000000000 R09: 1ffff110354dff46
> [ 232.734168] R10: fffffbfff0cb921d R11: 00000000910da5ce R12: 1ffffffff0c1fcdd
> [ 232.734459] R13: 1ffffffff0c23f36 R14: ffff888171628040 R15: 0000000000000000
> [ 232.734861] FS: 00007fb4907f86c0(0000) GS:ffff888791f2c000(0000) knlGS:0000000000000000
> [ 232.735265] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 232.735548] CR2: 00007fb47840be00 CR3: 000000015e6dc000 CR4: 00000000000006f0
> [ 232.736031] Call Trace:
> [ 232.736273] <TASK>
> [ 232.736500] get_locked_pte+0x1f/0xa0
> [ 232.736878] insert_pfn+0x9f/0x350
> [ 232.737190] ? __pfx_pat_pagerange_is_ram+0x10/0x10
> [ 232.737614] ? __pfx_insert_pfn+0x10/0x10
> [ 232.737990] ? __pfx_css_rstat_updated+0x10/0x10
> [ 232.738281] ? __pfx_pfn_modify_allowed+0x10/0x10
> [ 232.738552] ? lookup_memtype+0x62/0x180
> [ 232.738761] vmf_insert_pfn_prot+0x14b/0x340
> [ 232.739012] ? __pfx_vmf_insert_pfn_prot+0x10/0x10
> [ 232.739247] ? __pfx___might_resched+0x10/0x10
> [ 232.739475] drm_gem_shmem_fault.cold+0x18/0x39
> [ 232.739677] ? rcu_read_unlock+0x20/0x70
> [ 232.739882] __do_fault+0x251/0x7b0
> [ 232.740028] do_fault+0x6e1/0xc00
> [ 232.740167] ? __lock_acquire+0x590/0xc40
> [ 232.740335] handle_pte_fault+0x439/0x760
> [ 232.740498] ? mtree_range_walk+0x252/0xae0
> [ 232.740669] ? __pfx_handle_pte_fault+0x10/0x10
> [ 232.740899] __handle_mm_fault+0xa02/0xf30
> [ 232.741066] ? __pfx___handle_mm_fault+0x10/0x10
> [ 232.741255] ? find_vma+0xa1/0x120
> [ 232.741403] handle_mm_fault+0x2bf/0x8f0
> [ 232.741564] do_user_addr_fault+0x2d3/0xed0
> [ 232.741736] ? trace_page_fault_user+0x1bf/0x240
> [ 232.741969] exc_page_fault+0x87/0x120
> [ 232.742124] asm_exc_page_fault+0x26/0x30
> [ 232.742288] RIP: 0033:0x7fb4d73ed546
> [ 232.742441] Code: 66 41 0f 6f fb 66 44 0f 6d dc 66 44 0f 6f c6 66 41 0f 6d f1 66 0f 6c fc 66 45 0f 6c c1 66 44 0f 6f c9 66 0f 6d ca 66 0f db f0 <66> 0f df 04 08 66 44 0f 6c ca 66 45 0f db c2 66 44 0f df 10 66 44
> [ 232.743193] RSP: 002b:00007fb4907f68a0 EFLAGS: 00010206
> [ 232.743565] RAX: 00007fb47840aa00 RBX: 00007fb4d73ec070 RCX: 0000000000001400
> [ 232.743871] RDX: 0000000000002800 RSI: 0000000000003c00 RDI: 0000000000000001
> [ 232.744150] RBP: 0000000000000004 R08: 0000000000001400 R09: 00007fb4d73ec060
> [ 232.744433] R10: 000055f0261a4288 R11: 00007fb4c013da40 R12: 0000000000000008
> [ 232.744712] R13: 0000000000000000 R14: 4332322132212110 R15: 0000000000000004
> [ 232.746616] </TASK>
> [ 232.746711] Modules linked in: nft_nat nft_masq veth bridge stp llc snd_seq_dummy snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore overlay rfkill nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables qrtr ppdev 9pnet_virtio 9pnet parport_pc i2c_piix4 netfs pcspkr parport i2c_smbus joydev sunrpc vfat fat loop dm_multipath nfnetlink vsock_loopback vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport zram lz4hc_compress vmw_vmci lz4_compress vsock e1000 bochs serio_raw ata_generic pata_acpi scsi_dh_rdac scsi_dh_emc scsi_dh_alua i2c_dev fuse qemu_fw_cfg
> [ 232.749308] ---[ end trace 0000000000000000 ]---
> [ 232.749507] RIP: 0010:walk_to_pmd+0x29e/0x3c0
> [ 232.749692] Code: d8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 48 89 ea 48 89 de 4c 89 f7 e8 ae 85 ff ff 85 c0 0f 84 1f fe ff ff 31 db eb d0 <0f> 0b 48 89 ea 48 89 de 4c 89 f7 e8 92 8b ff ff 85 c0 75 e8 48 b8
> [ 232.750428] RSP: 0000:ffff8881aa6ff9a8 EFLAGS: 00010282
> [ 232.750645] RAX: 8000000142e002e7 RBX: ffff8881433cae10 RCX: dffffc0000000000
> [ 232.750954] RDX: 0000000000000000 RSI: 00007fb47840b000 RDI: 8000000142e002e7
> [ 232.751232] RBP: 00007fb47840b000 R08: 0000000000000000 R09: 1ffff110354dff46
> [ 232.751514] R10: fffffbfff0cb921d R11: 00000000910da5ce R12: 1ffffffff0c1fcdd
> [ 232.751837] R13: 1ffffffff0c23f36 R14: ffff888171628040 R15: 0000000000000000
> [ 232.752124] FS: 00007fb4907f86c0(0000) GS:ffff888791f2c000(0000) knlGS:0000000000000000
> [ 232.752441] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 232.752674] CR2: 00007fb47840be00 CR3: 000000015e6dc000 CR4: 00000000000006f0
> [ 232.752983] Kernel panic - not syncing: Fatal exception
> [ 232.753510] Kernel Offset: disabled
> [ 232.754643] ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> This happens when two concurrent page faults occur within the same PMD range.
> One fault installs a PMD mapping through vmf_insert_pfn_pmd(), while the other
> attempts to install a PTE mapping via vmf_insert_pfn(). The bug is
> triggered because a pmd_trans_huge is not expected when walking the page
> table inside vmf_insert_pfn.
>
> Avoid this race by adding a huge_fault callback to drm_gem_shmem_vm_ops so that
> PMD-sized mappings are handled through the appropriate huge page fault path.
>
> Fixes: 211b9a39f261 ("drm/shmem-helper: Map huge pages in fault handler")
> Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
> ---
>
> Changes in v2:
> - Keep the #ifdef unindented
> - Create drm_gem_shmem_any_fault to handle faults of any order and use
> drm_gem_shmem_[huge_]fault() as wrappers
>
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 67 +++++++++++++++-----------
> 1 file changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 7b5a49935ae4..fb8b74ac3057 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -550,36 +550,19 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
>
> -static bool drm_gem_shmem_try_map_pmd(struct vm_fault *vmf, unsigned long addr,
> - struct page *page)
> -{
> -#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
> - unsigned long pfn = page_to_pfn(page);
> - unsigned long paddr = pfn << PAGE_SHIFT;
> - bool aligned = (addr & ~PMD_MASK) == (paddr & ~PMD_MASK);
> -
> - if (aligned &&
> - pmd_none(*vmf->pmd) &&
> - folio_test_pmd_mappable(page_folio(page))) {
> - pfn &= PMD_MASK >> PAGE_SHIFT;
> - if (vmf_insert_pfn_pmd(vmf, pfn, false) == VM_FAULT_NOPAGE)
> - return true;
> - }
> -#endif
> -
> - return false;
> -}
> -
> -static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> +static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf,
> + unsigned int order)
> {
> struct vm_area_struct *vma = vmf->vma;
> struct drm_gem_object *obj = vma->vm_private_data;
> struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> loff_t num_pages = obj->size >> PAGE_SHIFT;
> - vm_fault_t ret;
> + vm_fault_t ret = VM_FAULT_FALLBACK;
> struct page **pages = shmem->pages;
> pgoff_t page_offset;
> unsigned long pfn;
> + unsigned long paddr;
> + bool aligned;
>
> /* Offset to faulty address in the VMA. */
> page_offset = vmf->pgoff - vma->vm_pgoff;
> @@ -593,13 +576,24 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> goto out;
> }
>
> - if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, pages[page_offset])) {
> - ret = VM_FAULT_NOPAGE;
> - goto out;
> - }
> -
> pfn = page_to_pfn(pages[page_offset]);
> - ret = vmf_insert_pfn(vma, vmf->address, pfn);
> + switch (order) {
> + case 0:
> + ret = vmf_insert_pfn(vma, vmf->address, pfn);
> + break;
> +#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
> + case PMD_ORDER:
> + paddr = pfn << PAGE_SHIFT;
> + aligned = (vmf->address & ~PMD_MASK) == (paddr & ~PMD_MASK);
> +
> + if (aligned &&
> + folio_test_pmd_mappable(page_folio(pages[page_offset]))) {
> + pfn &= PMD_MASK >> PAGE_SHIFT;
> + ret = vmf_insert_pfn_pmd(vmf, pfn, false);
> + }
> + break;
> +#endif
> + }
I'd rather have the pmd vs pte detection done early (before the lock is
taken) so we can return before acquiring the lock and we can get rid of
the order != PMD_ORDER check in drm_gem_shmem_huge_fault(). That, or we
keep the check in drm_gem_shmem_huge_fault() and pass a try_pmd boolean
instead of order to drm_gem_shmem_any_fault().
>
> out:
> dma_resv_unlock(shmem->base.resv);
> @@ -607,6 +601,20 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> return ret;
> }
>
> +static vm_fault_t drm_gem_shmem_huge_fault(struct vm_fault *vmf,
> + unsigned int order)
> +{
> + if (order != PMD_ORDER)
> + return VM_FAULT_FALLBACK;
> +
> + return drm_gem_shmem_any_fault(vmf, order);
> +}
> +
> +static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> +{
> + return drm_gem_shmem_any_fault(vmf, 0);
> +}
> +
> static void drm_gem_shmem_vm_open(struct vm_area_struct *vma)
> {
> struct drm_gem_object *obj = vma->vm_private_data;
> @@ -643,6 +651,9 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
>
> const struct vm_operations_struct drm_gem_shmem_vm_ops = {
> .fault = drm_gem_shmem_fault,
> +#ifdef CONFIG_ARCH_SUPPORTS_HUGE_PFNMAP
> + .huge_fault = drm_gem_shmem_huge_fault,
> +#endif
> .open = drm_gem_shmem_vm_open,
> .close = drm_gem_shmem_vm_close,
> };
^ permalink raw reply [flat|nested] 6+ messages in thread* Claude review: drm/shmem-helper: Fix Map huge page mapping in fault handler
2026-03-13 14:17 [PATCH v2] drm/shmem-helper: Fix Map huge page mapping in fault handler Pedro Demarchi Gomes
2026-03-13 17:08 ` Boris Brezillon
@ 2026-03-13 20:52 ` Claude Code Review Bot
2026-03-13 20:52 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-13 20:52 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/shmem-helper: Fix Map huge page mapping in fault handler
Author: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
Patches: 2
Reviewed: 2026-03-14T06:52:13.837152
---
This is a single-patch fix for a real race condition in `drm_gem_shmem_helper.c`. The bug occurs when two concurrent page faults within the same PMD range cause one fault to install a PMD mapping via `vmf_insert_pfn_pmd()` from the `.fault` handler, while another tries to install a PTE mapping via `vmf_insert_pfn()`. The kernel then hits a `BUG()` in `walk_to_pmd()` because it encounters an unexpected `pmd_trans_huge` entry.
The fix correctly separates the PMD mapping path into a `.huge_fault` callback, letting the mm subsystem properly route faults of different orders. The approach follows the same pattern already used by the VFIO PCI drivers (`vfio_pci_core.c`, `nvgrace-gpu/main.c`). The fix is appropriate and the Fixes tag is correct.
There are a couple of minor issues worth noting.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: drm/shmem-helper: Fix Map huge page mapping in fault handler
2026-03-13 14:17 [PATCH v2] drm/shmem-helper: Fix Map huge page mapping in fault handler Pedro Demarchi Gomes
2026-03-13 17:08 ` Boris Brezillon
2026-03-13 20:52 ` Claude review: " Claude Code Review Bot
@ 2026-03-13 20:52 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-13 20:52 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness of the fix:** The approach is sound. By moving the PMD mapping to `.huge_fault`, the mm subsystem handles the dispatch between PTE-level and PMD-level faults, eliminating the race. This is the correct way to do it — the old code was essentially doing huge page mapping opportunistically from the regular `.fault` path, which is inherently racy.
**Unused variable warning when `CONFIG_ARCH_SUPPORTS_PMD_PFNMAP` is not set:**
```c
static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf,
unsigned int order)
{
...
unsigned long paddr;
bool aligned;
```
The variables `paddr` and `aligned` are declared at function scope but only used inside the `#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP` block in `case PMD_ORDER:`. When that config is disabled, these variables are unused and will produce compiler warnings (`-Wunused-variable`). They should be moved inside the `case PMD_ORDER:` block (with braces around the case body), or `__maybe_unused` could be applied, though moving them is cleaner.
That said, in practice `drm_gem_shmem_any_fault` is only called with `order != 0` from `drm_gem_shmem_huge_fault`, which is only compiled when `CONFIG_ARCH_SUPPORTS_HUGE_PFNMAP` is set, which implies `CONFIG_ARCH_SUPPORTS_PMD_PFNMAP` is also set (per `mm/Kconfig`). So the code is functionally correct, but the warning is still a build issue for configurations without these options enabled (where `.fault` still calls `drm_gem_shmem_any_fault(vmf, 0)` and the variables are dead code).
**Dropped `pmd_none()` check:**
The old code had:
```c
if (aligned &&
pmd_none(*vmf->pmd) &&
folio_test_pmd_mappable(page_folio(page))) {
```
The new code drops the `pmd_none(*vmf->pmd)` check:
```c
if (aligned &&
folio_test_pmd_mappable(page_folio(pages[page_offset]))) {
pfn &= PMD_MASK >> PAGE_SHIFT;
ret = vmf_insert_pfn_pmd(vmf, pfn, false);
}
```
This is fine. In the old code, the `pmd_none` check was needed because the code was being called from the regular `.fault` path where the PMD might already be populated with PTEs. In the new `.huge_fault` path, the mm subsystem ensures we're only called when a PMD-level mapping is appropriate, so `vmf_insert_pfn_pmd()` can handle any existing PMD state internally (returning an appropriate error/fallback if needed).
**Minor style nit — commit subject:** "Fix Map huge page mapping" has an odd capitalisation of "Map". Consider: "Fix huge page mapping race in fault handler".
**Missing `default:` case in switch:**
```c
switch (order) {
case 0:
ret = vmf_insert_pfn(vma, vmf->address, pfn);
break;
#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
case PMD_ORDER:
...
break;
#endif
}
```
There's no `default:` case. If an unexpected order is passed, `ret` remains `VM_FAULT_FALLBACK` (from initialization), which is the right behavior, but a `default: break;` would be clearer and would silence any `-Wswitch` warnings if new enum values were ever relevant. The VFIO version (`vfio_pci_vmf_insert_pfn`) explicitly has a `default: return VM_FAULT_FALLBACK;`. This is very minor since `drm_gem_shmem_huge_fault` already filters for `PMD_ORDER` only.
**Overall:** Good fix for a real bug. The pattern matches existing in-tree usage (VFIO). The unused variable issue should be fixed before merging, but otherwise the patch looks correct.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] drm/shmem-helper: Fix Map huge page mapping in fault handler
@ 2026-03-16 0:26 Pedro Demarchi Gomes
2026-03-16 1:46 ` Claude review: " Claude Code Review Bot
2026-03-16 1:46 ` Claude Code Review Bot
0 siblings, 2 replies; 6+ messages in thread
From: Pedro Demarchi Gomes @ 2026-03-16 0:26 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Boris Brezillon
Cc: dri-devel, linux-kernel, Pedro Demarchi Gomes
When running ./tools/testing/selftests/mm/split_huge_page_test multiple
times with /sys/kernel/mm/transparent_hugepage/shmem_enabled and
/sys/kernel/mm/transparent_hugepage/enabled set as always the following BUG
occurs:
[ 232.728858] ------------[ cut here ]------------
[ 232.729458] kernel BUG at mm/memory.c:2276!
[ 232.729726] Oops: invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 232.730217] CPU: 19 UID: 60578 PID: 1497 Comm: llvmpipe-9 Not tainted 7.0.0-rc1mm-new+ #19 PREEMPT(lazy)
[ 232.730855] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-9.fc43 06/10/2025
[ 232.731360] RIP: 0010:walk_to_pmd+0x29e/0x3c0
[ 232.731569] Code: d8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 48 89 ea 48 89 de 4c 89 f7 e8 ae 85 ff ff 85 c0 0f 84 1f fe ff ff 31 db eb d0 <0f> 0b 48 89 ea 48 89 de 4c 89 f7 e8 92 8b ff ff 85 c0 75 e8 48 b8
[ 232.732614] RSP: 0000:ffff8881aa6ff9a8 EFLAGS: 00010282
[ 232.732991] RAX: 8000000142e002e7 RBX: ffff8881433cae10 RCX: dffffc0000000000
[ 232.733362] RDX: 0000000000000000 RSI: 00007fb47840b000 RDI: 8000000142e002e7
[ 232.733801] RBP: 00007fb47840b000 R08: 0000000000000000 R09: 1ffff110354dff46
[ 232.734168] R10: fffffbfff0cb921d R11: 00000000910da5ce R12: 1ffffffff0c1fcdd
[ 232.734459] R13: 1ffffffff0c23f36 R14: ffff888171628040 R15: 0000000000000000
[ 232.734861] FS: 00007fb4907f86c0(0000) GS:ffff888791f2c000(0000) knlGS:0000000000000000
[ 232.735265] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 232.735548] CR2: 00007fb47840be00 CR3: 000000015e6dc000 CR4: 00000000000006f0
[ 232.736031] Call Trace:
[ 232.736273] <TASK>
[ 232.736500] get_locked_pte+0x1f/0xa0
[ 232.736878] insert_pfn+0x9f/0x350
[ 232.737190] ? __pfx_pat_pagerange_is_ram+0x10/0x10
[ 232.737614] ? __pfx_insert_pfn+0x10/0x10
[ 232.737990] ? __pfx_css_rstat_updated+0x10/0x10
[ 232.738281] ? __pfx_pfn_modify_allowed+0x10/0x10
[ 232.738552] ? lookup_memtype+0x62/0x180
[ 232.738761] vmf_insert_pfn_prot+0x14b/0x340
[ 232.739012] ? __pfx_vmf_insert_pfn_prot+0x10/0x10
[ 232.739247] ? __pfx___might_resched+0x10/0x10
[ 232.739475] drm_gem_shmem_fault.cold+0x18/0x39
[ 232.739677] ? rcu_read_unlock+0x20/0x70
[ 232.739882] __do_fault+0x251/0x7b0
[ 232.740028] do_fault+0x6e1/0xc00
[ 232.740167] ? __lock_acquire+0x590/0xc40
[ 232.740335] handle_pte_fault+0x439/0x760
[ 232.740498] ? mtree_range_walk+0x252/0xae0
[ 232.740669] ? __pfx_handle_pte_fault+0x10/0x10
[ 232.740899] __handle_mm_fault+0xa02/0xf30
[ 232.741066] ? __pfx___handle_mm_fault+0x10/0x10
[ 232.741255] ? find_vma+0xa1/0x120
[ 232.741403] handle_mm_fault+0x2bf/0x8f0
[ 232.741564] do_user_addr_fault+0x2d3/0xed0
[ 232.741736] ? trace_page_fault_user+0x1bf/0x240
[ 232.741969] exc_page_fault+0x87/0x120
[ 232.742124] asm_exc_page_fault+0x26/0x30
[ 232.742288] RIP: 0033:0x7fb4d73ed546
[ 232.742441] Code: 66 41 0f 6f fb 66 44 0f 6d dc 66 44 0f 6f c6 66 41 0f 6d f1 66 0f 6c fc 66 45 0f 6c c1 66 44 0f 6f c9 66 0f 6d ca 66 0f db f0 <66> 0f df 04 08 66 44 0f 6c ca 66 45 0f db c2 66 44 0f df 10 66 44
[ 232.743193] RSP: 002b:00007fb4907f68a0 EFLAGS: 00010206
[ 232.743565] RAX: 00007fb47840aa00 RBX: 00007fb4d73ec070 RCX: 0000000000001400
[ 232.743871] RDX: 0000000000002800 RSI: 0000000000003c00 RDI: 0000000000000001
[ 232.744150] RBP: 0000000000000004 R08: 0000000000001400 R09: 00007fb4d73ec060
[ 232.744433] R10: 000055f0261a4288 R11: 00007fb4c013da40 R12: 0000000000000008
[ 232.744712] R13: 0000000000000000 R14: 4332322132212110 R15: 0000000000000004
[ 232.746616] </TASK>
[ 232.746711] Modules linked in: nft_nat nft_masq veth bridge stp llc snd_seq_dummy snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore overlay rfkill nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables qrtr ppdev 9pnet_virtio 9pnet parport_pc i2c_piix4 netfs pcspkr parport i2c_smbus joydev sunrpc vfat fat loop dm_multipath nfnetlink vsock_loopback vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport zram lz4hc_compress vmw_vmci lz4_compress vsock e1000 bochs serio_raw ata_generic pata_acpi scsi_dh_rdac scsi_dh_emc scsi_dh_alua i2c_dev fuse qemu_fw_cfg
[ 232.749308] ---[ end trace 0000000000000000 ]---
[ 232.749507] RIP: 0010:walk_to_pmd+0x29e/0x3c0
[ 232.749692] Code: d8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 48 89 ea 48 89 de 4c 89 f7 e8 ae 85 ff ff 85 c0 0f 84 1f fe ff ff 31 db eb d0 <0f> 0b 48 89 ea 48 89 de 4c 89 f7 e8 92 8b ff ff 85 c0 75 e8 48 b8
[ 232.750428] RSP: 0000:ffff8881aa6ff9a8 EFLAGS: 00010282
[ 232.750645] RAX: 8000000142e002e7 RBX: ffff8881433cae10 RCX: dffffc0000000000
[ 232.750954] RDX: 0000000000000000 RSI: 00007fb47840b000 RDI: 8000000142e002e7
[ 232.751232] RBP: 00007fb47840b000 R08: 0000000000000000 R09: 1ffff110354dff46
[ 232.751514] R10: fffffbfff0cb921d R11: 00000000910da5ce R12: 1ffffffff0c1fcdd
[ 232.751837] R13: 1ffffffff0c23f36 R14: ffff888171628040 R15: 0000000000000000
[ 232.752124] FS: 00007fb4907f86c0(0000) GS:ffff888791f2c000(0000) knlGS:0000000000000000
[ 232.752441] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 232.752674] CR2: 00007fb47840be00 CR3: 000000015e6dc000 CR4: 00000000000006f0
[ 232.752983] Kernel panic - not syncing: Fatal exception
[ 232.753510] Kernel Offset: disabled
[ 232.754643] ---[ end Kernel panic - not syncing: Fatal exception ]---
This happens when two concurrent page faults occur within the same PMD range.
One fault installs a PMD mapping through vmf_insert_pfn_pmd(), while the other
attempts to install a PTE mapping via vmf_insert_pfn(). The bug is
triggered because a pmd_trans_huge is not expected when walking the page
table inside vmf_insert_pfn.
Avoid this race by adding a huge_fault callback to drm_gem_shmem_vm_ops so that
PMD-sized mappings are handled through the appropriate huge page fault path.
Fixes: 211b9a39f261 ("drm/shmem-helper: Map huge pages in fault handler")
Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
---
Changes in v3:
- Pass a try_pmd boolean parameter to drm_gem_shmem_any_fault
- Compile drm_gem_shmem_huge_fault only if CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
is defined to avoid a build warning
Changes in v2: https://lore.kernel.org/dri-devel/20260313141719.3949700-1-pedrodemargomes@gmail.com/
- Keep the #ifdef unindented
- Create drm_gem_shmem_any_fault to handle faults of any order and use
drm_gem_shmem_[huge_]fault() as wrappers
v1: https://lore.kernel.org/all/20260312155027.1682606-1-pedrodemargomes@gmail.com/
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 63 ++++++++++++++------------
1 file changed, 35 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 7b5a49935ae4..9428c3ab7283 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -550,36 +550,18 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
-static bool drm_gem_shmem_try_map_pmd(struct vm_fault *vmf, unsigned long addr,
- struct page *page)
-{
-#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
- unsigned long pfn = page_to_pfn(page);
- unsigned long paddr = pfn << PAGE_SHIFT;
- bool aligned = (addr & ~PMD_MASK) == (paddr & ~PMD_MASK);
-
- if (aligned &&
- pmd_none(*vmf->pmd) &&
- folio_test_pmd_mappable(page_folio(page))) {
- pfn &= PMD_MASK >> PAGE_SHIFT;
- if (vmf_insert_pfn_pmd(vmf, pfn, false) == VM_FAULT_NOPAGE)
- return true;
- }
-#endif
-
- return false;
-}
-
-static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
+static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf, bool try_pmd)
{
struct vm_area_struct *vma = vmf->vma;
struct drm_gem_object *obj = vma->vm_private_data;
struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
loff_t num_pages = obj->size >> PAGE_SHIFT;
- vm_fault_t ret;
+ vm_fault_t ret = VM_FAULT_FALLBACK;
struct page **pages = shmem->pages;
pgoff_t page_offset;
unsigned long pfn;
+ unsigned long paddr;
+ bool aligned;
/* Offset to faulty address in the VMA. */
page_offset = vmf->pgoff - vma->vm_pgoff;
@@ -593,13 +575,19 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
goto out;
}
- if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, pages[page_offset])) {
- ret = VM_FAULT_NOPAGE;
- goto out;
- }
-
pfn = page_to_pfn(pages[page_offset]);
- ret = vmf_insert_pfn(vma, vmf->address, pfn);
+ if (try_pmd) {
+ paddr = pfn << PAGE_SHIFT;
+ aligned = (vmf->address & ~PMD_MASK) == (paddr & ~PMD_MASK);
+
+ if (aligned &&
+ folio_test_pmd_mappable(page_folio(pages[page_offset]))) {
+ pfn &= PMD_MASK >> PAGE_SHIFT;
+ ret = vmf_insert_pfn_pmd(vmf, pfn, false);
+ }
+ } else {
+ ret = vmf_insert_pfn(vma, vmf->address, pfn);
+ }
out:
dma_resv_unlock(shmem->base.resv);
@@ -607,6 +595,22 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
return ret;
}
+#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
+static vm_fault_t drm_gem_shmem_huge_fault(struct vm_fault *vmf,
+ unsigned int order)
+{
+ if (order != PMD_ORDER)
+ return VM_FAULT_FALLBACK;
+
+ return drm_gem_shmem_any_fault(vmf, true);
+}
+#endif
+
+static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
+{
+ return drm_gem_shmem_any_fault(vmf, false);
+}
+
static void drm_gem_shmem_vm_open(struct vm_area_struct *vma)
{
struct drm_gem_object *obj = vma->vm_private_data;
@@ -643,6 +647,9 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
const struct vm_operations_struct drm_gem_shmem_vm_ops = {
.fault = drm_gem_shmem_fault,
+#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
+ .huge_fault = drm_gem_shmem_huge_fault,
+#endif
.open = drm_gem_shmem_vm_open,
.close = drm_gem_shmem_vm_close,
};
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Claude review: drm/shmem-helper: Fix Map huge page mapping in fault handler
2026-03-16 0:26 [PATCH v3] " Pedro Demarchi Gomes
@ 2026-03-16 1:46 ` Claude Code Review Bot
2026-03-16 1:46 ` Claude Code Review Bot
1 sibling, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-16 1:46 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/shmem-helper: Fix Map huge page mapping in fault handler
Author: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
Patches: 1
Reviewed: 2026-03-16T11:46:49.216342
---
This is a single-patch fix for a real race condition in the DRM shmem helper's huge page fault handling. The bug occurs when two concurrent page faults in the same PMD range race: one installs a PMD mapping via `vmf_insert_pfn_pmd()`, while the other tries a PTE mapping via `vmf_insert_pfn()`, hitting a BUG in `walk_to_pmd()` because a `pmd_trans_huge` entry is unexpected in the PTE path.
The fix is architecturally correct: move PMD mapping to a proper `.huge_fault` callback and keep `.fault` for PTE-only mappings, letting the MM layer coordinate between the two paths. However, the patch does not apply to the current drm-next tree, which has already refactored this code (renaming functions, adding `folio_mark_accessed()`, adding `pfn_mkwrite`). The patch needs to be rebased.
**Verdict:** The approach is sound, but needs rebasing against drm-next and has a few issues noted below.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: drm/shmem-helper: Fix Map huge page mapping in fault handler
2026-03-16 0:26 [PATCH v3] " Pedro Demarchi Gomes
2026-03-16 1:46 ` Claude review: " Claude Code Review Bot
@ 2026-03-16 1:46 ` Claude Code Review Bot
1 sibling, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-16 1:46 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Race condition analysis is correct.** The commit message clearly explains the problem and reproducer. The Fixes tag is appropriate.
**Issue 1: Missing fallback from huge_fault to PTE mapping.**
When `.huge_fault` is called and the folio is PMD-mappable but alignment fails, the function returns `VM_FAULT_FALLBACK`. The MM layer will then call `.fault` which does PTE insertion. This is correct. However, if the folio is PMD-mappable and aligned but `vmf_insert_pfn_pmd()` itself fails (returns something other than `VM_FAULT_NOPAGE`), the error is propagated correctly too. This path looks fine.
**Issue 2: `pmd_none()` check was dropped.**
The original code had:
```c
if (aligned &&
pmd_none(*vmf->pmd) &&
folio_test_pmd_mappable(page_folio(page))) {
```
The new code removes the `pmd_none(*vmf->pmd)` check:
```c
if (aligned &&
folio_test_pmd_mappable(page_folio(pages[page_offset]))) {
```
In the `.huge_fault` path, the PMD should be guaranteed to be in a suitable state by the MM layer, so dropping this check is likely fine — `vmf_insert_pfn_pmd()` handles it internally. But it's worth confirming this is intentional and that `vmf_insert_pfn_pmd` handles the non-none PMD case gracefully (returning an appropriate error rather than corrupting the page table).
**Issue 3: Needs rebase on drm-next.**
The current drm-next tree at `drivers/gpu/drm/drm_gem_shmem_helper.c:557-612` has substantially different code:
- The helper is now called `drm_gem_shmem_try_insert_pfn_pmd()` (not `drm_gem_shmem_try_map_pmd`)
- There's a `folio_mark_accessed()` call on success (line 605-606)
- A `pfn_mkwrite` handler has been added (line 648-664)
- Variable declarations differ (`page`, `folio` locals; `drm_dev` used for warnings)
The patch needs to be rebased to target the current tree.
**Issue 4: `folio_mark_accessed()` is missing from the patched code.**
The current drm-next code calls `folio_mark_accessed(folio)` on successful mapping. The patch (based on an older tree) does not include this. If this patch is meant to be backported to the older tree that didn't have it, this is fine; but when rebased on drm-next, it should be preserved.
**Minor: Subject line capitalization.** "Fix Map huge page" — the "Map" should be lowercase: "Fix map huge page mapping in fault handler" or perhaps rephrased to "Fix race between PMD and PTE fault paths".
**Overall:** The fix is correct in approach. Using `.huge_fault` for PMD-order mappings and `.fault` for PTE-order is the right way to let the MM layer serialize concurrent faults at different granularities. The patch needs a rebase onto drm-next before it can be applied.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-03-16 1:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-13 14:17 [PATCH v2] drm/shmem-helper: Fix Map huge page mapping in fault handler Pedro Demarchi Gomes
2026-03-13 17:08 ` Boris Brezillon
2026-03-13 20:52 ` Claude review: " Claude Code Review Bot
2026-03-13 20:52 ` Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-03-16 0:26 [PATCH v3] " Pedro Demarchi Gomes
2026-03-16 1:46 ` Claude review: " Claude Code Review Bot
2026-03-16 1:46 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox