Hi,
thanks for reviewing.
Am 27.01.26 um 14:58 schrieb Matthew Wilcox:
On Tue, Jan 27, 2026 at 02:16:36PM +0100, Thomas Zimmermann wrote:
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -553,17 +553,18 @@ 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;
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+ phys_addr_t paddr = page_to_phys(page);
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;
+ if (aligned && pmd_none(*vmf->pmd)) {
+ struct folio *folio = page_folio(page);
+
+ if (folio_test_pmd_mappable(folio)) {
+ /* Read-only mapping; split upon write fault */
+ if (vmf_insert_folio_pmd(vmf, folio, false) ==
VM_FAULT_NOPAGE)
+ return true;
+ }
}
It would feel more natural to me to have this function return vm_fault_t
instead of a bool. What do you think?
I took what was already there, but I had the same thought.
+ pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within
VMA */
+ struct page *page = pages[page_offset];
+ vm_fault_t ret;
[...]
- if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, pages[page_offset])) {
- ret = VM_FAULT_NOPAGE;
- goto out;
+ page = pages[page_offset];
Can page_offset have changed at this point?
No, that's a mistake in the patch. This line is left over from an
earlier prototype. Apologies.
+ if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, page)) {
+ ret = VM_FAULT_NOPAGE;
+ } else {
+ struct folio *folio = page_folio(page);
+
+ get_page(page);
folio_get(folio);
- vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
+ vm_flags_mod(vma, VM_DONTEXPAND | VM_DONTDUMP, VM_PFNMAP);
Just so you know, this is going to change observable behaviour, maybe
in a good way. For example, we'll now be able to do O_DIRECT I/O to
this range and ptrace accesses. I think everything will be fine, but
VM_PFNMAP does prohibit some things which are now allowed.
Out of curiosity: how does this interact with the hugetable support?
Using any of these new features would implicitly disable them for that
mapping?
Best regards
Thomas
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
