* [PATCH v3 1/6] drm/gem-shmem: Use obj directly where appropriate in fault handler
2026-02-09 13:27 [PATCH v3 0/6] drm/gem-shmem: Track page accessed/dirty status Thomas Zimmermann
@ 2026-02-09 13:27 ` Thomas Zimmermann
2026-02-09 14:10 ` Boris Brezillon
2026-02-09 13:27 ` [PATCH v3 2/6] drm/gem-shmem: Test for existence of page in mmap " Thomas Zimmermann
` (4 subsequent siblings)
5 siblings, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-09 13:27 UTC (permalink / raw)
To: boris.brezillon, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, linux-mm, Thomas Zimmermann
Replace shmem->base with obj in several places. It is the same value,
but the latter is easier to read.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 3871a6d92f77..5bced7db0f1f 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -584,7 +584,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
/* Offset to faulty address in the VMA. */
page_offset = vmf->pgoff - vma->vm_pgoff;
- dma_resv_lock(shmem->base.resv, NULL);
+ dma_resv_lock(obj->resv, NULL);
if (page_offset >= num_pages ||
drm_WARN_ON_ONCE(obj->dev, !shmem->pages) ||
@@ -602,7 +602,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
ret = vmf_insert_pfn(vma, vmf->address, pfn);
out:
- dma_resv_unlock(shmem->base.resv);
+ dma_resv_unlock(obj->resv);
return ret;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 1/6] drm/gem-shmem: Use obj directly where appropriate in fault handler
2026-02-09 13:27 ` [PATCH v3 1/6] drm/gem-shmem: Use obj directly where appropriate in fault handler Thomas Zimmermann
@ 2026-02-09 14:10 ` Boris Brezillon
0 siblings, 0 replies; 30+ messages in thread
From: Boris Brezillon @ 2026-02-09 14:10 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
On Mon, 9 Feb 2026 14:27:10 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Replace shmem->base with obj in several places. It is the same value,
> but the latter is easier to read.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 3871a6d92f77..5bced7db0f1f 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -584,7 +584,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> /* Offset to faulty address in the VMA. */
> page_offset = vmf->pgoff - vma->vm_pgoff;
>
> - dma_resv_lock(shmem->base.resv, NULL);
> + dma_resv_lock(obj->resv, NULL);
>
> if (page_offset >= num_pages ||
> drm_WARN_ON_ONCE(obj->dev, !shmem->pages) ||
> @@ -602,7 +602,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>
> out:
> - dma_resv_unlock(shmem->base.resv);
> + dma_resv_unlock(obj->resv);
>
> return ret;
> }
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 2/6] drm/gem-shmem: Test for existence of page in mmap fault handler
2026-02-09 13:27 [PATCH v3 0/6] drm/gem-shmem: Track page accessed/dirty status Thomas Zimmermann
2026-02-09 13:27 ` [PATCH v3 1/6] drm/gem-shmem: Use obj directly where appropriate in fault handler Thomas Zimmermann
@ 2026-02-09 13:27 ` Thomas Zimmermann
2026-02-09 14:10 ` Boris Brezillon
2026-02-09 13:27 ` [PATCH v3 3/6] drm/gem-shmem: Return vm_fault_t from drm_gem_shmem_try_map_pmd() Thomas Zimmermann
` (3 subsequent siblings)
5 siblings, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-09 13:27 UTC (permalink / raw)
To: boris.brezillon, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, linux-mm, Thomas Zimmermann
Not having a page pointer in the mmap fault handler is an error. Test
for this situation and return VM_FAULT_SIGBUS if so. Also replace several
lookups of the page with a local variable.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 5bced7db0f1f..3ee54c24e535 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -574,31 +574,31 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
struct drm_gem_object *obj = vma->vm_private_data;
+ struct drm_device *dev = obj->dev;
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_SIGBUS;
struct page **pages = shmem->pages;
- pgoff_t page_offset;
+ pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
+ struct page *page;
unsigned long pfn;
- /* Offset to faulty address in the VMA. */
- page_offset = vmf->pgoff - vma->vm_pgoff;
-
dma_resv_lock(obj->resv, NULL);
- if (page_offset >= num_pages ||
- drm_WARN_ON_ONCE(obj->dev, !shmem->pages) ||
- shmem->madv < 0) {
- ret = VM_FAULT_SIGBUS;
+ if (page_offset >= num_pages || drm_WARN_ON_ONCE(dev, !shmem->pages) ||
+ shmem->madv < 0)
+ goto out;
+
+ page = pages[page_offset];
+ if (drm_WARN_ON_ONCE(dev, !page))
goto out;
- }
- if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, pages[page_offset])) {
+ if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, page)) {
ret = VM_FAULT_NOPAGE;
goto out;
}
- pfn = page_to_pfn(pages[page_offset]);
+ pfn = page_to_pfn(page);
ret = vmf_insert_pfn(vma, vmf->address, pfn);
out:
--
2.52.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 2/6] drm/gem-shmem: Test for existence of page in mmap fault handler
2026-02-09 13:27 ` [PATCH v3 2/6] drm/gem-shmem: Test for existence of page in mmap " Thomas Zimmermann
@ 2026-02-09 14:10 ` Boris Brezillon
0 siblings, 0 replies; 30+ messages in thread
From: Boris Brezillon @ 2026-02-09 14:10 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
On Mon, 9 Feb 2026 14:27:11 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Not having a page pointer in the mmap fault handler is an error. Test
> for this situation and return VM_FAULT_SIGBUS if so. Also replace several
> lookups of the page with a local variable.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 5bced7db0f1f..3ee54c24e535 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -574,31 +574,31 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> {
> struct vm_area_struct *vma = vmf->vma;
> struct drm_gem_object *obj = vma->vm_private_data;
> + struct drm_device *dev = obj->dev;
> 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_SIGBUS;
> struct page **pages = shmem->pages;
> - pgoff_t page_offset;
> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
> + struct page *page;
> unsigned long pfn;
>
> - /* Offset to faulty address in the VMA. */
> - page_offset = vmf->pgoff - vma->vm_pgoff;
> -
> dma_resv_lock(obj->resv, NULL);
>
> - if (page_offset >= num_pages ||
> - drm_WARN_ON_ONCE(obj->dev, !shmem->pages) ||
> - shmem->madv < 0) {
> - ret = VM_FAULT_SIGBUS;
> + if (page_offset >= num_pages || drm_WARN_ON_ONCE(dev, !shmem->pages) ||
> + shmem->madv < 0)
> + goto out;
> +
> + page = pages[page_offset];
> + if (drm_WARN_ON_ONCE(dev, !page))
> goto out;
> - }
>
> - if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, pages[page_offset])) {
> + if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, page)) {
> ret = VM_FAULT_NOPAGE;
> goto out;
> }
>
> - pfn = page_to_pfn(pages[page_offset]);
> + pfn = page_to_pfn(page);
> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>
> out:
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 3/6] drm/gem-shmem: Return vm_fault_t from drm_gem_shmem_try_map_pmd()
2026-02-09 13:27 [PATCH v3 0/6] drm/gem-shmem: Track page accessed/dirty status Thomas Zimmermann
2026-02-09 13:27 ` [PATCH v3 1/6] drm/gem-shmem: Use obj directly where appropriate in fault handler Thomas Zimmermann
2026-02-09 13:27 ` [PATCH v3 2/6] drm/gem-shmem: Test for existence of page in mmap " Thomas Zimmermann
@ 2026-02-09 13:27 ` Thomas Zimmermann
2026-02-09 13:27 ` [PATCH v3 4/6] drm/gem-shmem: Refactor drm_gem_shmem_try_map_pmd() Thomas Zimmermann
` (2 subsequent siblings)
5 siblings, 0 replies; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-09 13:27 UTC (permalink / raw)
To: boris.brezillon, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, linux-mm, Thomas Zimmermann
Return the exact VM_FAULT_ mask from drm_gem_shmem_try_map_pmd(). Gives
the caller better insight into the result. Return 0 if nothing was done.
If the caller sees VM_FAULT_NOPAGE, drm_gem_shmem_try_map_pmd() added a
PMD entry to the page table. As before, return early from the page-fault
handler in that case.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Suggested-by: Matthew Wilcox <willy@infradead.org>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 3ee54c24e535..ab8e331005f9 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -550,8 +550,8 @@ 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)
+static vm_fault_t 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);
@@ -562,12 +562,11 @@ static bool drm_gem_shmem_try_map_pmd(struct vm_fault *vmf, unsigned long addr,
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;
+ return vmf_insert_pfn_pmd(vmf, pfn, false);
}
#endif
- return false;
+ return 0;
}
static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
@@ -593,10 +592,9 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
if (drm_WARN_ON_ONCE(dev, !page))
goto out;
- if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, page)) {
- ret = VM_FAULT_NOPAGE;
+ ret = drm_gem_shmem_try_map_pmd(vmf, vmf->address, page);
+ if (ret == VM_FAULT_NOPAGE)
goto out;
- }
pfn = page_to_pfn(page);
ret = vmf_insert_pfn(vma, vmf->address, pfn);
--
2.52.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v3 4/6] drm/gem-shmem: Refactor drm_gem_shmem_try_map_pmd()
2026-02-09 13:27 [PATCH v3 0/6] drm/gem-shmem: Track page accessed/dirty status Thomas Zimmermann
` (2 preceding siblings ...)
2026-02-09 13:27 ` [PATCH v3 3/6] drm/gem-shmem: Return vm_fault_t from drm_gem_shmem_try_map_pmd() Thomas Zimmermann
@ 2026-02-09 13:27 ` Thomas Zimmermann
2026-02-09 14:25 ` Boris Brezillon
2026-02-09 13:27 ` [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap Thomas Zimmermann
2026-02-09 13:27 ` [PATCH v3 6/6] drm/gem-shmem: Track folio accessed/dirty status in vmap Thomas Zimmermann
5 siblings, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-09 13:27 UTC (permalink / raw)
To: boris.brezillon, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, linux-mm, Thomas Zimmermann
The current mmap page-fault handler requires some changes before it
can track folio access.
Call to folio_test_pmd_mappable() into the mmap page-fault handler
before calling drm_gem_shmem_try_map_pmd(). The folio will become
useful for tracking the access status.
Also rename drm_gem_shmem_try_map_pmd() to _try_insert_pfn_pmd()
and only pass the page fault and page-frame number. The new name and
parameters make it similar to vmf_insert_pfn_pmd().
No functional changes. If PMD mapping fails or is not supported,
insert a regular PFN as before.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index ab8e331005f9..c3a054899ba3 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -550,17 +550,14 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
-static vm_fault_t drm_gem_shmem_try_map_pmd(struct vm_fault *vmf, unsigned long addr,
- struct page *page)
+static vm_fault_t drm_gem_shmem_try_insert_pfn_pmd(struct vm_fault *vmf, unsigned long pfn)
{
#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);
+ bool aligned = (vmf->address & ~PMD_MASK) == (paddr & ~PMD_MASK);
- if (aligned &&
- pmd_none(*vmf->pmd) &&
- folio_test_pmd_mappable(page_folio(page))) {
+ if (aligned && pmd_none(*vmf->pmd)) {
+ /* Read-only mapping; split upon write fault */
pfn &= PMD_MASK >> PAGE_SHIFT;
return vmf_insert_pfn_pmd(vmf, pfn, false);
}
@@ -580,6 +577,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
struct page **pages = shmem->pages;
pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
struct page *page;
+ struct folio *folio;
unsigned long pfn;
dma_resv_lock(obj->resv, NULL);
@@ -591,15 +589,16 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
page = pages[page_offset];
if (drm_WARN_ON_ONCE(dev, !page))
goto out;
-
- ret = drm_gem_shmem_try_map_pmd(vmf, vmf->address, page);
- if (ret == VM_FAULT_NOPAGE)
- goto out;
+ folio = page_folio(page);
pfn = page_to_pfn(page);
- ret = vmf_insert_pfn(vma, vmf->address, pfn);
- out:
+ if (folio_test_pmd_mappable(folio))
+ ret = drm_gem_shmem_try_insert_pfn_pmd(vmf, pfn);
+ if (ret != VM_FAULT_NOPAGE)
+ ret = vmf_insert_pfn(vma, vmf->address, pfn);
+
+out:
dma_resv_unlock(obj->resv);
return ret;
--
2.52.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 4/6] drm/gem-shmem: Refactor drm_gem_shmem_try_map_pmd()
2026-02-09 13:27 ` [PATCH v3 4/6] drm/gem-shmem: Refactor drm_gem_shmem_try_map_pmd() Thomas Zimmermann
@ 2026-02-09 14:25 ` Boris Brezillon
0 siblings, 0 replies; 30+ messages in thread
From: Boris Brezillon @ 2026-02-09 14:25 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
On Mon, 9 Feb 2026 14:27:13 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> The current mmap page-fault handler requires some changes before it
> can track folio access.
>
> Call to folio_test_pmd_mappable() into the mmap page-fault handler
> before calling drm_gem_shmem_try_map_pmd(). The folio will become
> useful for tracking the access status.
>
> Also rename drm_gem_shmem_try_map_pmd() to _try_insert_pfn_pmd()
> and only pass the page fault and page-frame number. The new name and
> parameters make it similar to vmf_insert_pfn_pmd().
>
> No functional changes. If PMD mapping fails or is not supported,
> insert a regular PFN as before.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index ab8e331005f9..c3a054899ba3 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -550,17 +550,14 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
>
> -static vm_fault_t drm_gem_shmem_try_map_pmd(struct vm_fault *vmf, unsigned long addr,
> - struct page *page)
> +static vm_fault_t drm_gem_shmem_try_insert_pfn_pmd(struct vm_fault *vmf, unsigned long pfn)
> {
> #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);
> + bool aligned = (vmf->address & ~PMD_MASK) == (paddr & ~PMD_MASK);
>
> - if (aligned &&
> - pmd_none(*vmf->pmd) &&
> - folio_test_pmd_mappable(page_folio(page))) {
> + if (aligned && pmd_none(*vmf->pmd)) {
> + /* Read-only mapping; split upon write fault */
> pfn &= PMD_MASK >> PAGE_SHIFT;
> return vmf_insert_pfn_pmd(vmf, pfn, false);
> }
> @@ -580,6 +577,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> struct page **pages = shmem->pages;
> pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
> struct page *page;
> + struct folio *folio;
> unsigned long pfn;
>
> dma_resv_lock(obj->resv, NULL);
> @@ -591,15 +589,16 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> page = pages[page_offset];
> if (drm_WARN_ON_ONCE(dev, !page))
> goto out;
> -
> - ret = drm_gem_shmem_try_map_pmd(vmf, vmf->address, page);
> - if (ret == VM_FAULT_NOPAGE)
> - goto out;
> + folio = page_folio(page);
>
> pfn = page_to_pfn(page);
> - ret = vmf_insert_pfn(vma, vmf->address, pfn);
>
> - out:
> + if (folio_test_pmd_mappable(folio))
> + ret = drm_gem_shmem_try_insert_pfn_pmd(vmf, pfn);
> + if (ret != VM_FAULT_NOPAGE)
> + ret = vmf_insert_pfn(vma, vmf->address, pfn);
> +
> +out:
> dma_resv_unlock(obj->resv);
>
> return ret;
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-02-09 13:27 [PATCH v3 0/6] drm/gem-shmem: Track page accessed/dirty status Thomas Zimmermann
` (3 preceding siblings ...)
2026-02-09 13:27 ` [PATCH v3 4/6] drm/gem-shmem: Refactor drm_gem_shmem_try_map_pmd() Thomas Zimmermann
@ 2026-02-09 13:27 ` Thomas Zimmermann
2026-02-09 14:23 ` Boris Brezillon
2026-05-20 13:11 ` Igor Torrente
2026-02-09 13:27 ` [PATCH v3 6/6] drm/gem-shmem: Track folio accessed/dirty status in vmap Thomas Zimmermann
5 siblings, 2 replies; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-09 13:27 UTC (permalink / raw)
To: boris.brezillon, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, linux-mm, Thomas Zimmermann
Invoke folio_mark_accessed() in mmap page faults to add the folio to
the memory manager's LRU list. Userspace invokes mmap to get the memory
for software rendering. Compositors do the same when creating the final
on-screen image, so keeping the pages in LRU makes sense. Avoids paging
out graphics buffers when under memory pressure.
In pfn_mkwrite, further invoke the folio_mark_dirty() to add the folio
for writeback should the underlying file be paged out from system memory.
This rarely happens in practice, yet it would corrupt the buffer content.
This has little effect on a system's hardware-accelerated rendering, which
only mmaps for an initial setup of textures, meshes, shaders, etc.
v3:
- rewrite for VM_PFNMAP
v2:
- adapt to changes in drm_gem_shmem_try_mmap_pmd()
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index c3a054899ba3..0c86ad40a049 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
if (ret != VM_FAULT_NOPAGE)
ret = vmf_insert_pfn(vma, vmf->address, pfn);
+ if (likely(!(ret & VM_FAULT_ERROR)))
+ folio_mark_accessed(folio);
+
out:
dma_resv_unlock(obj->resv);
@@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
drm_gem_vm_close(vma);
}
+static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
+{
+ 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);
+ pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
+ struct page *page = shmem->pages[page_offset];
+ struct folio *folio = page_folio(page);
+
+ file_update_time(vma->vm_file);
+
+ folio_mark_dirty(folio);
+
+ return 0;
+}
+
const struct vm_operations_struct drm_gem_shmem_vm_ops = {
.fault = drm_gem_shmem_fault,
.open = drm_gem_shmem_vm_open,
.close = drm_gem_shmem_vm_close,
+ .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
};
EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
--
2.52.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-02-09 13:27 ` [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap Thomas Zimmermann
@ 2026-02-09 14:23 ` Boris Brezillon
2026-02-09 14:46 ` Thomas Zimmermann
2026-05-20 13:11 ` Igor Torrente
1 sibling, 1 reply; 30+ messages in thread
From: Boris Brezillon @ 2026-02-09 14:23 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
On Mon, 9 Feb 2026 14:27:14 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Invoke folio_mark_accessed() in mmap page faults to add the folio to
> the memory manager's LRU list. Userspace invokes mmap to get the memory
> for software rendering. Compositors do the same when creating the final
> on-screen image, so keeping the pages in LRU makes sense. Avoids paging
> out graphics buffers when under memory pressure.
>
> In pfn_mkwrite, further invoke the folio_mark_dirty() to add the folio
> for writeback should the underlying file be paged out from system memory.
> This rarely happens in practice, yet it would corrupt the buffer content.
>
> This has little effect on a system's hardware-accelerated rendering, which
> only mmaps for an initial setup of textures, meshes, shaders, etc.
>
> v3:
> - rewrite for VM_PFNMAP
> v2:
> - adapt to changes in drm_gem_shmem_try_mmap_pmd()
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index c3a054899ba3..0c86ad40a049 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> if (ret != VM_FAULT_NOPAGE)
> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>
> + if (likely(!(ret & VM_FAULT_ERROR)))
Can't we just go
if (ret == VM_FAULT_NOPAGE)
here?
> + folio_mark_accessed(folio);
> +
> out:
> dma_resv_unlock(obj->resv);
>
> @@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
> drm_gem_vm_close(vma);
> }
>
> +static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
> +{
> + 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);
> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
> + struct page *page = shmem->pages[page_offset];
Should we have a
if (WARN_ON(!shmem->pages ||
page_offset <= (obj->size >> PAGE_SHIFT)))
return VM_FAULT_SIGBUS;
?
> + struct folio *folio = page_folio(page);
> +
> + file_update_time(vma->vm_file);
> +
> + folio_mark_dirty(folio);
> +
> + return 0;
> +}
> +
> const struct vm_operations_struct drm_gem_shmem_vm_ops = {
> .fault = drm_gem_shmem_fault,
> .open = drm_gem_shmem_vm_open,
> .close = drm_gem_shmem_vm_close,
> + .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
> };
> EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-02-09 14:23 ` Boris Brezillon
@ 2026-02-09 14:46 ` Thomas Zimmermann
2026-02-09 15:01 ` Boris Brezillon
0 siblings, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-09 14:46 UTC (permalink / raw)
To: Boris Brezillon
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
Hi Boris,
thanks for reviewing the series.
Am 09.02.26 um 15:23 schrieb Boris Brezillon:
> On Mon, 9 Feb 2026 14:27:14 +0100
> Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>> Invoke folio_mark_accessed() in mmap page faults to add the folio to
>> the memory manager's LRU list. Userspace invokes mmap to get the memory
>> for software rendering. Compositors do the same when creating the final
>> on-screen image, so keeping the pages in LRU makes sense. Avoids paging
>> out graphics buffers when under memory pressure.
>>
>> In pfn_mkwrite, further invoke the folio_mark_dirty() to add the folio
>> for writeback should the underlying file be paged out from system memory.
>> This rarely happens in practice, yet it would corrupt the buffer content.
>>
>> This has little effect on a system's hardware-accelerated rendering, which
>> only mmaps for an initial setup of textures, meshes, shaders, etc.
>>
>> v3:
>> - rewrite for VM_PFNMAP
>> v2:
>> - adapt to changes in drm_gem_shmem_try_mmap_pmd()
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
>> ---
>> drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index c3a054899ba3..0c86ad40a049 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
>> if (ret != VM_FAULT_NOPAGE)
>> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>>
>> + if (likely(!(ret & VM_FAULT_ERROR)))
> Can't we just go
>
> if (ret == VM_FAULT_NOPAGE)
>
> here?
After reviewing the code in vmf_insert_pfn, I think so. All we'll see is
_OOM and _SIGBUS; or _NOPAGE on success. I'll change it then.
>
>> + folio_mark_accessed(folio);
>> +
>> out:
>> dma_resv_unlock(obj->resv);
>>
>> @@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
>> drm_gem_vm_close(vma);
>> }
>>
>> +static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
>> +{
>> + 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);
>> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
>> + struct page *page = shmem->pages[page_offset];
> Should we have a
>
> if (WARN_ON(!shmem->pages ||
> page_offset <= (obj->size >> PAGE_SHIFT)))
> return VM_FAULT_SIGBUS;
>
>
> ?
I left it out because it doesn't seem necessary. In the fault handler
in drm_gem_shmem_fault(), I can see that we could get an OOB access. But
we only call pfn_mkwrite() after going through _fault() first. I don't
see a way of getting here unless we've already tested for the page in
_fault().
Best regards
Thomas
>
>> + struct folio *folio = page_folio(page);
>> +
>> + file_update_time(vma->vm_file);
>> +
>> + folio_mark_dirty(folio);
>> +
>> + return 0;
>> +}
>> +
>> const struct vm_operations_struct drm_gem_shmem_vm_ops = {
>> .fault = drm_gem_shmem_fault,
>> .open = drm_gem_shmem_vm_open,
>> .close = drm_gem_shmem_vm_close,
>> + .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
>> };
>> EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
>>
--
--
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)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-02-09 14:46 ` Thomas Zimmermann
@ 2026-02-09 15:01 ` Boris Brezillon
2026-02-25 11:35 ` Thomas Zimmermann
0 siblings, 1 reply; 30+ messages in thread
From: Boris Brezillon @ 2026-02-09 15:01 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
On Mon, 9 Feb 2026 15:46:21 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi Boris,
>
> thanks for reviewing the series.
>
> Am 09.02.26 um 15:23 schrieb Boris Brezillon:
> > On Mon, 9 Feb 2026 14:27:14 +0100
> > Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> >> Invoke folio_mark_accessed() in mmap page faults to add the folio to
> >> the memory manager's LRU list. Userspace invokes mmap to get the memory
> >> for software rendering. Compositors do the same when creating the final
> >> on-screen image, so keeping the pages in LRU makes sense. Avoids paging
> >> out graphics buffers when under memory pressure.
> >>
> >> In pfn_mkwrite, further invoke the folio_mark_dirty() to add the folio
> >> for writeback should the underlying file be paged out from system memory.
> >> This rarely happens in practice, yet it would corrupt the buffer content.
> >>
> >> This has little effect on a system's hardware-accelerated rendering, which
> >> only mmaps for an initial setup of textures, meshes, shaders, etc.
> >>
> >> v3:
> >> - rewrite for VM_PFNMAP
> >> v2:
> >> - adapt to changes in drm_gem_shmem_try_mmap_pmd()
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> >> ---
> >> drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
> >> 1 file changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> >> index c3a054899ba3..0c86ad40a049 100644
> >> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> >> @@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> >> if (ret != VM_FAULT_NOPAGE)
> >> ret = vmf_insert_pfn(vma, vmf->address, pfn);
> >>
> >> + if (likely(!(ret & VM_FAULT_ERROR)))
> > Can't we just go
> >
> > if (ret == VM_FAULT_NOPAGE)
> >
> > here?
>
> After reviewing the code in vmf_insert_pfn, I think so. All we'll see is
> _OOM and _SIGBUS; or _NOPAGE on success. I'll change it then.
>
>
> >
> >> + folio_mark_accessed(folio);
> >> +
> >> out:
> >> dma_resv_unlock(obj->resv);
> >>
> >> @@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
> >> drm_gem_vm_close(vma);
> >> }
> >>
> >> +static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
> >> +{
> >> + 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);
> >> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
> >> + struct page *page = shmem->pages[page_offset];
> > Should we have a
> >
> > if (WARN_ON(!shmem->pages ||
> > page_offset <= (obj->size >> PAGE_SHIFT)))
> > return VM_FAULT_SIGBUS;
> >
> >
> > ?
>
> I left it out because it doesn't seem necessary. In the fault handler
> in drm_gem_shmem_fault(), I can see that we could get an OOB access. But
> we only call pfn_mkwrite() after going through _fault() first. I don't
> see a way of getting here unless we've already tested for the page in
> _fault().
I agree it's not supposed to happen, but isn't it what WARN_ON()s are
for (catching unexpected situations)?
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-02-09 15:01 ` Boris Brezillon
@ 2026-02-25 11:35 ` Thomas Zimmermann
0 siblings, 0 replies; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-25 11:35 UTC (permalink / raw)
To: Boris Brezillon
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
Hi
Am 09.02.26 um 16:01 schrieb Boris Brezillon:
> On Mon, 9 Feb 2026 15:46:21 +0100
> Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>> Hi Boris,
>>
>> thanks for reviewing the series.
>>
>> Am 09.02.26 um 15:23 schrieb Boris Brezillon:
>>> On Mon, 9 Feb 2026 14:27:14 +0100
>>> Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>
>>>> Invoke folio_mark_accessed() in mmap page faults to add the folio to
>>>> the memory manager's LRU list. Userspace invokes mmap to get the memory
>>>> for software rendering. Compositors do the same when creating the final
>>>> on-screen image, so keeping the pages in LRU makes sense. Avoids paging
>>>> out graphics buffers when under memory pressure.
>>>>
>>>> In pfn_mkwrite, further invoke the folio_mark_dirty() to add the folio
>>>> for writeback should the underlying file be paged out from system memory.
>>>> This rarely happens in practice, yet it would corrupt the buffer content.
>>>>
>>>> This has little effect on a system's hardware-accelerated rendering, which
>>>> only mmaps for an initial setup of textures, meshes, shaders, etc.
>>>>
>>>> v3:
>>>> - rewrite for VM_PFNMAP
>>>> v2:
>>>> - adapt to changes in drm_gem_shmem_try_mmap_pmd()
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
>>>> ---
>>>> drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
>>>> 1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> index c3a054899ba3..0c86ad40a049 100644
>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> @@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
>>>> if (ret != VM_FAULT_NOPAGE)
>>>> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>>>>
>>>> + if (likely(!(ret & VM_FAULT_ERROR)))
>>> Can't we just go
>>>
>>> if (ret == VM_FAULT_NOPAGE)
>>>
>>> here?
>> After reviewing the code in vmf_insert_pfn, I think so. All we'll see is
>> _OOM and _SIGBUS; or _NOPAGE on success. I'll change it then.
>>
>>
>>>
>>>> + folio_mark_accessed(folio);
>>>> +
>>>> out:
>>>> dma_resv_unlock(obj->resv);
>>>>
>>>> @@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
>>>> drm_gem_vm_close(vma);
>>>> }
>>>>
>>>> +static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
>>>> +{
>>>> + 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);
>>>> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
>>>> + struct page *page = shmem->pages[page_offset];
>>> Should we have a
>>>
>>> if (WARN_ON(!shmem->pages ||
>>> page_offset <= (obj->size >> PAGE_SHIFT)))
>>> return VM_FAULT_SIGBUS;
>>>
>>>
>>> ?
>> I left it out because it doesn't seem necessary. In the fault handler
>> in drm_gem_shmem_fault(), I can see that we could get an OOB access. But
>> we only call pfn_mkwrite() after going through _fault() first. I don't
>> see a way of getting here unless we've already tested for the page in
>> _fault().
> I agree it's not supposed to happen, but isn't it what WARN_ON()s are
> for (catching unexpected situations)?
OK, with Frank's ack on the imagination changes in, I'll send out an
update with additional warning.
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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-02-09 13:27 ` [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap Thomas Zimmermann
2026-02-09 14:23 ` Boris Brezillon
@ 2026-05-20 13:11 ` Igor Torrente
2026-05-20 14:44 ` Boris Brezillon
2026-05-26 14:44 ` Thomas Zimmermann
1 sibling, 2 replies; 30+ messages in thread
From: Igor Torrente @ 2026-05-20 13:11 UTC (permalink / raw)
To: Thomas Zimmermann, boris.brezillon, loic.molinari, willy,
frank.binns, matt.coster, maarten.lankhorst, mripard, airlied,
simona
Cc: dri-devel, linux-mm
On 2/9/26 10:27, Thomas Zimmermann wrote:
> Invoke folio_mark_accessed() in mmap page faults to add the folio to
> the memory manager's LRU list. Userspace invokes mmap to get the memory
> for software rendering. Compositors do the same when creating the final
> on-screen image, so keeping the pages in LRU makes sense. Avoids paging
> out graphics buffers when under memory pressure.
>
> In pfn_mkwrite, further invoke the folio_mark_dirty() to add the folio
> for writeback should the underlying file be paged out from system memory.
> This rarely happens in practice, yet it would corrupt the buffer content.
>
> This has little effect on a system's hardware-accelerated rendering, which
> only mmaps for an initial setup of textures, meshes, shaders, etc.
>
> v3:
> - rewrite for VM_PFNMAP
> v2:
> - adapt to changes in drm_gem_shmem_try_mmap_pmd()
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Hi Thomas,
I have and Radxa ROCK 5B+ with Mali-G610 GPU and I'm developing the VDRM
(native context) driver
for panthor.ko.
Recently I update my kernel from 7.0 to the 7.1-rc3, and qemu started
freezing with KVM error below
when running any graphical application with Venus or Panthor VDRM. For
some reason Virgl is not affected.
```
error: kvm run failed Bad address
PC=0000ffffb4a86d70 X00=0000ffffb4be5000 X01=0000ffffb2279bb0
X02=0000000000000600 X03=0000ffffb4be5000 X04=0000ffffb227a1b0
X05=0000ffffb4be5600 X06=0000000000000001 X07=0000aaaaf384fd80
X08=dcc58d0daf897753 X09=0000000000000002 X10=0000ffffb4b96758
X11=000000002533f84b X12=0000000000001060 X13=0000000000000000
X14=0000000000000000 X15=0000000000000010 X16=0000ffffb3880128
X17=0000ffffb4a86c80 X18=0000000000000000 X19=0000aaaaf384f8a0
X20=0000aaaaf37cec80 X21=0000aaaaf37c7210 X22=0000aaaaf37c9550
X23=0000aaaaf37c74e0 X24=0000ffffb3b47fc8 X25=0000ffffb3b47fc8
X26=0000ffffb3b43c78 X27=0000aaaaf37e1840 X28=0000aaaaf37cee50
X29=0000ffffecbf1a60 X30=0000ffffb364b530 SP=0000ffffecbf1a60
PSTATE=0000000020001000 --C- EL0t
```
After bisecting I reach to your commit.
```
28e3918179aa75cfd4490b3b0281496ed91d829a drm/gem-shmem:
Track folio accessed/dirty status in mmap
```
It still reproducible in the 7.1-rc4 (ab5fce87a778).
Appling the following patch (Boris suggestion) fixes the issue for me.
```
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 545933c7f712..1e036f69c1ce 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -696,7 +696,7 @@ const struct vm_operations_struct
drm_gem_shmem_vm_ops = {
#endif
.open = drm_gem_shmem_vm_open,
.close = drm_gem_shmem_vm_close,
- .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
+// .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
};
EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
```
I'm not familiar with the drm_gem, code so I don't any more insightful
information
to share. But hopefully we can find a better fix for this.
BR,
Igor Torrente
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index c3a054899ba3..0c86ad40a049 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> if (ret != VM_FAULT_NOPAGE)
> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>
> + if (likely(!(ret & VM_FAULT_ERROR)))
> + folio_mark_accessed(folio);
> +
> out:
> dma_resv_unlock(obj->resv);
>
> @@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
> drm_gem_vm_close(vma);
> }
>
> +static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
> +{
> + 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);
> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
> + struct page *page = shmem->pages[page_offset];
> + struct folio *folio = page_folio(page);
> +
> + file_update_time(vma->vm_file);
> +
> + folio_mark_dirty(folio);
> +
> + return 0;
> +}
> +
> const struct vm_operations_struct drm_gem_shmem_vm_ops = {
> .fault = drm_gem_shmem_fault,
> .open = drm_gem_shmem_vm_open,
> .close = drm_gem_shmem_vm_close,
> + .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
> };
> EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
>
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-20 13:11 ` Igor Torrente
@ 2026-05-20 14:44 ` Boris Brezillon
2026-05-22 8:31 ` Thomas Zimmermann
2026-05-26 14:44 ` Thomas Zimmermann
1 sibling, 1 reply; 30+ messages in thread
From: Boris Brezillon @ 2026-05-20 14:44 UTC (permalink / raw)
To: Igor Torrente
Cc: Thomas Zimmermann, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona, dri-devel, linux-mm
On Wed, 20 May 2026 10:11:03 -0300
Igor Torrente <igor.torrente@collabora.com> wrote:
> On 2/9/26 10:27, Thomas Zimmermann wrote:
> > Invoke folio_mark_accessed() in mmap page faults to add the folio to
> > the memory manager's LRU list. Userspace invokes mmap to get the memory
> > for software rendering. Compositors do the same when creating the final
> > on-screen image, so keeping the pages in LRU makes sense. Avoids paging
> > out graphics buffers when under memory pressure.
> >
> > In pfn_mkwrite, further invoke the folio_mark_dirty() to add the folio
> > for writeback should the underlying file be paged out from system memory.
> > This rarely happens in practice, yet it would corrupt the buffer content.
> >
> > This has little effect on a system's hardware-accelerated rendering, which
> > only mmaps for an initial setup of textures, meshes, shaders, etc.
> >
> > v3:
> > - rewrite for VM_PFNMAP
> > v2:
> > - adapt to changes in drm_gem_shmem_try_mmap_pmd()
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
>
> Hi Thomas,
>
> I have and Radxa ROCK 5B+ with Mali-G610 GPU and I'm developing the VDRM
> (native context) driver
> for panthor.ko.
>
> Recently I update my kernel from 7.0 to the 7.1-rc3, and qemu started
> freezing with KVM error below
> when running any graphical application with Venus or Panthor VDRM. For
> some reason Virgl is not affected.
>
> ```
> error: kvm run failed Bad address
> PC=0000ffffb4a86d70 X00=0000ffffb4be5000 X01=0000ffffb2279bb0
> X02=0000000000000600 X03=0000ffffb4be5000 X04=0000ffffb227a1b0
> X05=0000ffffb4be5600 X06=0000000000000001 X07=0000aaaaf384fd80
> X08=dcc58d0daf897753 X09=0000000000000002 X10=0000ffffb4b96758
> X11=000000002533f84b X12=0000000000001060 X13=0000000000000000
> X14=0000000000000000 X15=0000000000000010 X16=0000ffffb3880128
> X17=0000ffffb4a86c80 X18=0000000000000000 X19=0000aaaaf384f8a0
> X20=0000aaaaf37cec80 X21=0000aaaaf37c7210 X22=0000aaaaf37c9550
> X23=0000aaaaf37c74e0 X24=0000ffffb3b47fc8 X25=0000ffffb3b47fc8
> X26=0000ffffb3b43c78 X27=0000aaaaf37e1840 X28=0000aaaaf37cee50
> X29=0000ffffecbf1a60 X30=0000ffffb364b530 SP=0000ffffecbf1a60
> PSTATE=0000000020001000 --C- EL0t
> ```
>
> After bisecting I reach to your commit.
>
> ```
> 28e3918179aa75cfd4490b3b0281496ed91d829a drm/gem-shmem:
> Track folio accessed/dirty status in mmap
> ```
>
> It still reproducible in the 7.1-rc4 (ab5fce87a778).
>
> Appling the following patch (Boris suggestion) fixes the issue for me.
>
> ```
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 545933c7f712..1e036f69c1ce 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -696,7 +696,7 @@ const struct vm_operations_struct
> drm_gem_shmem_vm_ops = {
> #endif
> .open = drm_gem_shmem_vm_open,
> .close = drm_gem_shmem_vm_close,
> - .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
> +// .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
> };
> EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
> ```
>
> I'm not familiar with the drm_gem, code so I don't any more insightful
> information
> to share. But hopefully we can find a better fix for this.
Hm, given how close we are from the end of the release cycle, it might
be safer to revert those .pfn_mkwrite changes and consider them again
once all the details have been sorted out (it's the second time around
we hit issues on this patch, and the lack of reviews from MM devs is
scaring me bit to be honest).
Thomas, any opinion?
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-20 14:44 ` Boris Brezillon
@ 2026-05-22 8:31 ` Thomas Zimmermann
0 siblings, 0 replies; 30+ messages in thread
From: Thomas Zimmermann @ 2026-05-22 8:31 UTC (permalink / raw)
To: Boris Brezillon, Igor Torrente
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
Hi
Am 20.05.26 um 16:44 schrieb Boris Brezillon:
> On Wed, 20 May 2026 10:11:03 -0300
> Igor Torrente <igor.torrente@collabora.com> wrote:
>
>> On 2/9/26 10:27, Thomas Zimmermann wrote:
>>> Invoke folio_mark_accessed() in mmap page faults to add the folio to
>>> the memory manager's LRU list. Userspace invokes mmap to get the memory
>>> for software rendering. Compositors do the same when creating the final
>>> on-screen image, so keeping the pages in LRU makes sense. Avoids paging
>>> out graphics buffers when under memory pressure.
>>>
>>> In pfn_mkwrite, further invoke the folio_mark_dirty() to add the folio
>>> for writeback should the underlying file be paged out from system memory.
>>> This rarely happens in practice, yet it would corrupt the buffer content.
>>>
>>> This has little effect on a system's hardware-accelerated rendering, which
>>> only mmaps for an initial setup of textures, meshes, shaders, etc.
>>>
>>> v3:
>>> - rewrite for VM_PFNMAP
>>> v2:
>>> - adapt to changes in drm_gem_shmem_try_mmap_pmd()
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
>> Hi Thomas,
>>
>> I have and Radxa ROCK 5B+ with Mali-G610 GPU and I'm developing the VDRM
>> (native context) driver
>> for panthor.ko.
>>
>> Recently I update my kernel from 7.0 to the 7.1-rc3, and qemu started
>> freezing with KVM error below
>> when running any graphical application with Venus or Panthor VDRM. For
>> some reason Virgl is not affected.
>>
>> ```
>> error: kvm run failed Bad address
>> PC=0000ffffb4a86d70 X00=0000ffffb4be5000 X01=0000ffffb2279bb0
>> X02=0000000000000600 X03=0000ffffb4be5000 X04=0000ffffb227a1b0
>> X05=0000ffffb4be5600 X06=0000000000000001 X07=0000aaaaf384fd80
>> X08=dcc58d0daf897753 X09=0000000000000002 X10=0000ffffb4b96758
>> X11=000000002533f84b X12=0000000000001060 X13=0000000000000000
>> X14=0000000000000000 X15=0000000000000010 X16=0000ffffb3880128
>> X17=0000ffffb4a86c80 X18=0000000000000000 X19=0000aaaaf384f8a0
>> X20=0000aaaaf37cec80 X21=0000aaaaf37c7210 X22=0000aaaaf37c9550
>> X23=0000aaaaf37c74e0 X24=0000ffffb3b47fc8 X25=0000ffffb3b47fc8
>> X26=0000ffffb3b43c78 X27=0000aaaaf37e1840 X28=0000aaaaf37cee50
>> X29=0000ffffecbf1a60 X30=0000ffffb364b530 SP=0000ffffecbf1a60
>> PSTATE=0000000020001000 --C- EL0t
>> ```
>>
>> After bisecting I reach to your commit.
>>
>> ```
>> 28e3918179aa75cfd4490b3b0281496ed91d829a drm/gem-shmem:
>> Track folio accessed/dirty status in mmap
>> ```
>>
>> It still reproducible in the 7.1-rc4 (ab5fce87a778).
>>
>> Appling the following patch (Boris suggestion) fixes the issue for me.
>>
>> ```
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index 545933c7f712..1e036f69c1ce 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -696,7 +696,7 @@ const struct vm_operations_struct
>> drm_gem_shmem_vm_ops = {
>> #endif
>> .open = drm_gem_shmem_vm_open,
>> .close = drm_gem_shmem_vm_close,
>> - .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
>> +// .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
>> };
>> EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
>> ```
>>
>> I'm not familiar with the drm_gem, code so I don't any more insightful
>> information
>> to share. But hopefully we can find a better fix for this.
> Hm, given how close we are from the end of the release cycle, it might
> be safer to revert those .pfn_mkwrite changes and consider them again
> once all the details have been sorted out (it's the second time around
> we hit issues on this patch, and the lack of reviews from MM devs is
> scaring me bit to be honest).
>
> Thomas, any opinion?
Sure, please go ahead. The mmap change in this series seems rather fragile.
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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-20 13:11 ` Igor Torrente
2026-05-20 14:44 ` Boris Brezillon
@ 2026-05-26 14:44 ` Thomas Zimmermann
2026-05-26 14:56 ` Igor Torrente
1 sibling, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-05-26 14:44 UTC (permalink / raw)
To: Igor Torrente, boris.brezillon, loic.molinari, willy, frank.binns,
matt.coster, maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, linux-mm
Hi
Am 20.05.26 um 15:11 schrieb Igor Torrente:
[...]
>
> I'm not familiar with the drm_gem, code so I don't any more insightful
> information
> to share. But hopefully we can find a better fix for this.
Do you have huge pages enabled ?
Best regards
Thomas
>
> BR,
> Igor Torrente
>
>> ---
>> drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index c3a054899ba3..0c86ad40a049 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct
>> vm_fault *vmf)
>> if (ret != VM_FAULT_NOPAGE)
>> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>> + if (likely(!(ret & VM_FAULT_ERROR)))
>> + folio_mark_accessed(folio);
>> +
>> out:
>> dma_resv_unlock(obj->resv);
>> @@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct
>> vm_area_struct *vma)
>> drm_gem_vm_close(vma);
>> }
>> +static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
>> +{
>> + 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);
>> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset
>> within VMA */
>> + struct page *page = shmem->pages[page_offset];
>> + struct folio *folio = page_folio(page);
>> +
>> + file_update_time(vma->vm_file);
>> +
>> + folio_mark_dirty(folio);
>> +
>> + return 0;
>> +}
>> +
>> const struct vm_operations_struct drm_gem_shmem_vm_ops = {
>> .fault = drm_gem_shmem_fault,
>> .open = drm_gem_shmem_vm_open,
>> .close = drm_gem_shmem_vm_close,
>> + .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
>> };
>> EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
--
--
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)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-26 14:44 ` Thomas Zimmermann
@ 2026-05-26 14:56 ` Igor Torrente
2026-05-27 6:56 ` Thomas Zimmermann
0 siblings, 1 reply; 30+ messages in thread
From: Igor Torrente @ 2026-05-26 14:56 UTC (permalink / raw)
To: Thomas Zimmermann, boris.brezillon, loic.molinari, willy,
frank.binns, matt.coster, maarten.lankhorst, mripard, airlied,
simona
Cc: dri-devel, linux-mm
Hi Thomas,
On 5/26/26 11:44, Thomas Zimmermann wrote:
> Hi
>
> Am 20.05.26 um 15:11 schrieb Igor Torrente:
> [...]
>>
>> I'm not familiar with the drm_gem, code so I don't any more
>> insightful information
>> to share. But hopefully we can find a better fix for this.
>
> Do you have huge pages enabled ?
No, I didn't even compile my kernel with huge pages. I've already been
burned by that before
BR,
Igor Torrente
>
> Best regards
> Thomas
>
>>
>> BR,
>> Igor Torrente
>>
>>> ---
>>> drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> index c3a054899ba3..0c86ad40a049 100644
>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> @@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct
>>> vm_fault *vmf)
>>> if (ret != VM_FAULT_NOPAGE)
>>> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>>> + if (likely(!(ret & VM_FAULT_ERROR)))
>>> + folio_mark_accessed(folio);
>>> +
>>> out:
>>> dma_resv_unlock(obj->resv);
>>> @@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct
>>> vm_area_struct *vma)
>>> drm_gem_vm_close(vma);
>>> }
>>> +static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
>>> +{
>>> + 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);
>>> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page
>>> offset within VMA */
>>> + struct page *page = shmem->pages[page_offset];
>>> + struct folio *folio = page_folio(page);
>>> +
>>> + file_update_time(vma->vm_file);
>>> +
>>> + folio_mark_dirty(folio);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> const struct vm_operations_struct drm_gem_shmem_vm_ops = {
>>> .fault = drm_gem_shmem_fault,
>>> .open = drm_gem_shmem_vm_open,
>>> .close = drm_gem_shmem_vm_close,
>>> + .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
>>> };
>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-26 14:56 ` Igor Torrente
@ 2026-05-27 6:56 ` Thomas Zimmermann
2026-05-27 10:18 ` Boris Brezillon
0 siblings, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-05-27 6:56 UTC (permalink / raw)
To: Igor Torrente, boris.brezillon, loic.molinari, willy, frank.binns,
matt.coster, maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, linux-mm
[-- Attachment #1: Type: text/plain, Size: 2777 bytes --]
Hi
Am 26.05.26 um 16:56 schrieb Igor Torrente:
> Hi Thomas,
>
> On 5/26/26 11:44, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 20.05.26 um 15:11 schrieb Igor Torrente:
>> [...]
>>>
>>> I'm not familiar with the drm_gem, code so I don't any more
>>> insightful information
>>> to share. But hopefully we can find a better fix for this.
>>
>> Do you have huge pages enabled ?
>
> No, I didn't even compile my kernel with huge pages. I've already been
> burned by that before
I see.
The attached patch is a cleaned up version of Boris' fix. Please test
and report back on the results.
Best regards
Thomas
>
>
> BR,
> Igor Torrente
>
>>
>> Best regards
>> Thomas
>>
>>>
>>> BR,
>>> Igor Torrente
>>>
>>>> ---
>>>> drivers/gpu/drm/drm_gem_shmem_helper.c | 20 ++++++++++++++++++++
>>>> 1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> index c3a054899ba3..0c86ad40a049 100644
>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> @@ -598,6 +598,9 @@ static vm_fault_t drm_gem_shmem_fault(struct
>>>> vm_fault *vmf)
>>>> if (ret != VM_FAULT_NOPAGE)
>>>> ret = vmf_insert_pfn(vma, vmf->address, pfn);
>>>> + if (likely(!(ret & VM_FAULT_ERROR)))
>>>> + folio_mark_accessed(folio);
>>>> +
>>>> out:
>>>> dma_resv_unlock(obj->resv);
>>>> @@ -638,10 +641,27 @@ static void drm_gem_shmem_vm_close(struct
>>>> vm_area_struct *vma)
>>>> drm_gem_vm_close(vma);
>>>> }
>>>> +static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
>>>> +{
>>>> + 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);
>>>> + pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page
>>>> offset within VMA */
>>>> + struct page *page = shmem->pages[page_offset];
>>>> + struct folio *folio = page_folio(page);
>>>> +
>>>> + file_update_time(vma->vm_file);
>>>> +
>>>> + folio_mark_dirty(folio);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> const struct vm_operations_struct drm_gem_shmem_vm_ops = {
>>>> .fault = drm_gem_shmem_fault,
>>>> .open = drm_gem_shmem_vm_open,
>>>> .close = drm_gem_shmem_vm_close,
>>>> + .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
>>>> };
>>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
>>
--
--
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)
[-- Attachment #2: 0001-drm-gem-shmem-Always-assume-writable-mmap-drop-pfn_m.patch --]
[-- Type: text/x-patch, Size: 3562 bytes --]
From 4478b3e07515194812806c5bfc3b7432d8f8406a Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Wed, 27 May 2026 08:07:19 +0200
Subject: [PATCH] drm/gem-shmem: Always assume writable mmap; drop pfn_mkwrite
Using pfn_mkwrite breaks KVM with "error: kvm run failed Bad address".
Seen on a Mali-G610 GPU. Fix this by always marking the mapped pages
as written.
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 46 +++++++-------------------
1 file changed, 12 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 545933c7f712..07e117673124 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -554,21 +554,6 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
-static void drm_gem_shmem_record_mkwrite(struct vm_fault *vmf)
-{
- 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;
- pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
-
- if (drm_WARN_ON(obj->dev, !shmem->pages || page_offset >= num_pages))
- return;
-
- file_update_time(vma->vm_file);
- folio_mark_dirty(page_folio(shmem->pages[page_offset]));
-}
-
static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order,
unsigned long pfn)
{
@@ -581,23 +566,16 @@ static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order,
if (aligned &&
folio_test_pmd_mappable(page_folio(pfn_to_page(pfn)))) {
- vm_fault_t ret;
-
pfn &= PMD_MASK >> PAGE_SHIFT;
- /* Unlike PTEs which are automatically upgraded to
+ /*
+ * Unlike PTEs which are automatically upgraded to
* writeable entries, the PMD upgrades go through
* .huge_fault(). Make sure we pass the "write" info
* along in that case.
- * This also means we have to record the write fault
- * here, instead of in .pfn_mkwrite().
*/
- ret = vmf_insert_pfn_pmd(vmf, pfn,
- vmf->flags & FAULT_FLAG_WRITE);
- if (ret == VM_FAULT_NOPAGE && (vmf->flags & FAULT_FLAG_WRITE))
- drm_gem_shmem_record_mkwrite(vmf);
-
- return ret;
+ return vmf_insert_pfn_pmd(vmf, pfn,
+ vmf->flags & FAULT_FLAG_WRITE);
}
#endif
}
@@ -635,8 +613,15 @@ static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf, unsigned int ord
pfn = page_to_pfn(page);
ret = try_insert_pfn(vmf, order, pfn);
- if (ret == VM_FAULT_NOPAGE)
+ if (ret == VM_FAULT_NOPAGE) {
folio_mark_accessed(folio);
+ /*
+ * Always record write access to the buffer. The natural
+ * place would be pfn_mkwrite, but this breaks KVM.
+ */
+ file_update_time(vma->vm_file);
+ folio_mark_dirty(folio);
+ }
out:
dma_resv_unlock(obj->resv);
@@ -683,12 +668,6 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
drm_gem_vm_close(vma);
}
-static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf)
-{
- drm_gem_shmem_record_mkwrite(vmf);
- return 0;
-}
-
const struct vm_operations_struct drm_gem_shmem_vm_ops = {
.fault = drm_gem_shmem_fault,
#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
@@ -696,7 +675,6 @@ const struct vm_operations_struct drm_gem_shmem_vm_ops = {
#endif
.open = drm_gem_shmem_vm_open,
.close = drm_gem_shmem_vm_close,
- .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite,
};
EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
--
2.54.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-27 6:56 ` Thomas Zimmermann
@ 2026-05-27 10:18 ` Boris Brezillon
2026-05-27 10:32 ` Thomas Zimmermann
2026-05-28 7:20 ` Thomas Zimmermann
0 siblings, 2 replies; 30+ messages in thread
From: Boris Brezillon @ 2026-05-27 10:18 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Igor Torrente, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona, dri-devel, linux-mm
Hello Thomas,
I'm inlining the diff you posted so I can comment on it directly before
it's officially posted to the list.
On Wed, 27 May 2026 08:56:33 +0200
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 545933c7f712..07e117673124 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -554,21 +554,6 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
>
> -static void drm_gem_shmem_record_mkwrite(struct vm_fault *vmf)
> -{
> - 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;
> - pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
> -
> - if (drm_WARN_ON(obj->dev, !shmem->pages || page_offset >= num_pages))
> - return;
> -
> - file_update_time(vma->vm_file);
> - folio_mark_dirty(page_folio(shmem->pages[page_offset]));
> -}
> -
> static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order,
> unsigned long pfn)
> {
> @@ -581,23 +566,16 @@ static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order,
>
> if (aligned &&
> folio_test_pmd_mappable(page_folio(pfn_to_page(pfn)))) {
> - vm_fault_t ret;
> -
> pfn &= PMD_MASK >> PAGE_SHIFT;
>
> - /* Unlike PTEs which are automatically upgraded to
> + /*
> + * Unlike PTEs which are automatically upgraded to
> * writeable entries, the PMD upgrades go through
> * .huge_fault(). Make sure we pass the "write" info
> * along in that case.
> - * This also means we have to record the write fault
> - * here, instead of in .pfn_mkwrite().
> */
> - ret = vmf_insert_pfn_pmd(vmf, pfn,
> - vmf->flags & FAULT_FLAG_WRITE);
> - if (ret == VM_FAULT_NOPAGE && (vmf->flags & FAULT_FLAG_WRITE))
> - drm_gem_shmem_record_mkwrite(vmf);
> -
> - return ret;
> + return vmf_insert_pfn_pmd(vmf, pfn,
> + vmf->flags & FAULT_FLAG_WRITE);
I believe we can go back to
return vmf_insert_pfn_pmd(vmf, pfn, false);
if the mappings are no longer adjusted to catch write accesses.
> }
> #endif
> }
> @@ -635,8 +613,15 @@ static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf, unsigned int ord
> pfn = page_to_pfn(page);
>
> ret = try_insert_pfn(vmf, order, pfn);
> - if (ret == VM_FAULT_NOPAGE)
> + if (ret == VM_FAULT_NOPAGE) {
> folio_mark_accessed(folio);
> + /*
> + * Always record write access to the buffer. The natural
> + * place would be pfn_mkwrite, but this breaks KVM.
> + */
> + file_update_time(vma->vm_file);
> + folio_mark_dirty(folio);
We can be a bit smarter here:
/*
* Always record write access to the buffer if the
* mapping is writeable. The natural place would be
* pfn_mkwrite, but this breaks KVM.
*/
if (vma->vm_flags & VM_WRITE) {
file_update_time(vma->vm_file);
folio_mark_dirty(folio);
}
> + }
The rest looks good to me.
Regards,
Boris
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-27 10:18 ` Boris Brezillon
@ 2026-05-27 10:32 ` Thomas Zimmermann
2026-05-27 15:02 ` Igor Torrente
2026-05-28 7:20 ` Thomas Zimmermann
1 sibling, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-05-27 10:32 UTC (permalink / raw)
To: Boris Brezillon
Cc: Igor Torrente, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona, dri-devel, linux-mm
Hi
Am 27.05.26 um 12:18 schrieb Boris Brezillon:
> Hello Thomas,
>
> I'm inlining the diff you posted so I can comment on it directly before
> it's officially posted to the list.
Thanks, I'll incorporate those in the next test rev or the official patch.
Best regards
Thomas
>
> On Wed, 27 May 2026 08:56:33 +0200
> Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index 545933c7f712..07e117673124 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -554,21 +554,6 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
>> }
>> EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
>>
>> -static void drm_gem_shmem_record_mkwrite(struct vm_fault *vmf)
>> -{
>> - 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;
>> - pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
>> -
>> - if (drm_WARN_ON(obj->dev, !shmem->pages || page_offset >= num_pages))
>> - return;
>> -
>> - file_update_time(vma->vm_file);
>> - folio_mark_dirty(page_folio(shmem->pages[page_offset]));
>> -}
>> -
>> static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order,
>> unsigned long pfn)
>> {
>> @@ -581,23 +566,16 @@ static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order,
>>
>> if (aligned &&
>> folio_test_pmd_mappable(page_folio(pfn_to_page(pfn)))) {
>> - vm_fault_t ret;
>> -
>> pfn &= PMD_MASK >> PAGE_SHIFT;
>>
>> - /* Unlike PTEs which are automatically upgraded to
>> + /*
>> + * Unlike PTEs which are automatically upgraded to
>> * writeable entries, the PMD upgrades go through
>> * .huge_fault(). Make sure we pass the "write" info
>> * along in that case.
>> - * This also means we have to record the write fault
>> - * here, instead of in .pfn_mkwrite().
>> */
>> - ret = vmf_insert_pfn_pmd(vmf, pfn,
>> - vmf->flags & FAULT_FLAG_WRITE);
>> - if (ret == VM_FAULT_NOPAGE && (vmf->flags & FAULT_FLAG_WRITE))
>> - drm_gem_shmem_record_mkwrite(vmf);
>> -
>> - return ret;
>> + return vmf_insert_pfn_pmd(vmf, pfn,
>> + vmf->flags & FAULT_FLAG_WRITE);
> I believe we can go back to
>
> return vmf_insert_pfn_pmd(vmf, pfn, false);
>
> if the mappings are no longer adjusted to catch write accesses.
>
>> }
>> #endif
>> }
>> @@ -635,8 +613,15 @@ static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf, unsigned int ord
>> pfn = page_to_pfn(page);
>>
>> ret = try_insert_pfn(vmf, order, pfn);
>> - if (ret == VM_FAULT_NOPAGE)
>> + if (ret == VM_FAULT_NOPAGE) {
>> folio_mark_accessed(folio);
>> + /*
>> + * Always record write access to the buffer. The natural
>> + * place would be pfn_mkwrite, but this breaks KVM.
>> + */
>> + file_update_time(vma->vm_file);
>> + folio_mark_dirty(folio);
> We can be a bit smarter here:
>
> /*
> * Always record write access to the buffer if the
> * mapping is writeable. The natural place would be
> * pfn_mkwrite, but this breaks KVM.
> */
> if (vma->vm_flags & VM_WRITE) {
> file_update_time(vma->vm_file);
> folio_mark_dirty(folio);
> }
>
>> + }
> The rest looks good to me.
>
> Regards,
>
> Boris
--
--
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)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-27 10:32 ` Thomas Zimmermann
@ 2026-05-27 15:02 ` Igor Torrente
2026-05-27 15:19 ` Thomas Zimmermann
0 siblings, 1 reply; 30+ messages in thread
From: Igor Torrente @ 2026-05-27 15:02 UTC (permalink / raw)
To: Thomas Zimmermann, Boris Brezillon
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
Hi Tomas,
I tested your patch and Boris's modifications on top of your patch, and
I'm happy to report that both seem to work just fine.
BR,
Igor Torrente
On 5/27/26 07:32, Thomas Zimmermann wrote:
> Hi
>
> Am 27.05.26 um 12:18 schrieb Boris Brezillon:
>> Hello Thomas,
>>
>> I'm inlining the diff you posted so I can comment on it directly before
>> it's officially posted to the list.
>
> Thanks, I'll incorporate those in the next test rev or the official
> patch.
>
> Best regards
> Thomas
>
>>
>> On Wed, 27 May 2026 08:56:33 +0200
>> Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> index 545933c7f712..07e117673124 100644
>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> @@ -554,21 +554,6 @@ int drm_gem_shmem_dumb_create(struct drm_file
>>> *file, struct drm_device *dev,
>>> }
>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
>>> -static void drm_gem_shmem_record_mkwrite(struct vm_fault *vmf)
>>> -{
>>> - 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;
>>> - pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page
>>> offset within VMA */
>>> -
>>> - if (drm_WARN_ON(obj->dev, !shmem->pages || page_offset >=
>>> num_pages))
>>> - return;
>>> -
>>> - file_update_time(vma->vm_file);
>>> - folio_mark_dirty(page_folio(shmem->pages[page_offset]));
>>> -}
>>> -
>>> static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned
>>> int order,
>>> unsigned long pfn)
>>> {
>>> @@ -581,23 +566,16 @@ static vm_fault_t try_insert_pfn(struct
>>> vm_fault *vmf, unsigned int order,
>>> if (aligned &&
>>> folio_test_pmd_mappable(page_folio(pfn_to_page(pfn)))) {
>>> - vm_fault_t ret;
>>> -
>>> pfn &= PMD_MASK >> PAGE_SHIFT;
>>> - /* Unlike PTEs which are automatically upgraded to
>>> + /*
>>> + * Unlike PTEs which are automatically upgraded to
>>> * writeable entries, the PMD upgrades go through
>>> * .huge_fault(). Make sure we pass the "write" info
>>> * along in that case.
>>> - * This also means we have to record the write fault
>>> - * here, instead of in .pfn_mkwrite().
>>> */
>>> - ret = vmf_insert_pfn_pmd(vmf, pfn,
>>> - vmf->flags & FAULT_FLAG_WRITE);
>>> - if (ret == VM_FAULT_NOPAGE && (vmf->flags &
>>> FAULT_FLAG_WRITE))
>>> - drm_gem_shmem_record_mkwrite(vmf);
>>> -
>>> - return ret;
>>> + return vmf_insert_pfn_pmd(vmf, pfn,
>>> + vmf->flags & FAULT_FLAG_WRITE);
>> I believe we can go back to
>>
>> return vmf_insert_pfn_pmd(vmf, pfn, false);
>>
>> if the mappings are no longer adjusted to catch write accesses.
>>
>>> }
>>> #endif
>>> }
>>> @@ -635,8 +613,15 @@ static vm_fault_t
>>> drm_gem_shmem_any_fault(struct vm_fault *vmf, unsigned int ord
>>> pfn = page_to_pfn(page);
>>> ret = try_insert_pfn(vmf, order, pfn);
>>> - if (ret == VM_FAULT_NOPAGE)
>>> + if (ret == VM_FAULT_NOPAGE) {
>>> folio_mark_accessed(folio);
>>> + /*
>>> + * Always record write access to the buffer. The natural
>>> + * place would be pfn_mkwrite, but this breaks KVM.
>>> + */
>>> + file_update_time(vma->vm_file);
>>> + folio_mark_dirty(folio);
>> We can be a bit smarter here:
>>
>> /*
>> * Always record write access to the buffer if the
>> * mapping is writeable. The natural place would be
>> * pfn_mkwrite, but this breaks KVM.
>> */
>> if (vma->vm_flags & VM_WRITE) {
>> file_update_time(vma->vm_file);
>> folio_mark_dirty(folio);
>> }
>>
>>> + }
>> The rest looks good to me.
>>
>> Regards,
>>
>> Boris
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-27 15:02 ` Igor Torrente
@ 2026-05-27 15:19 ` Thomas Zimmermann
0 siblings, 0 replies; 30+ messages in thread
From: Thomas Zimmermann @ 2026-05-27 15:19 UTC (permalink / raw)
To: Igor Torrente, Boris Brezillon
Cc: loic.molinari, willy, frank.binns, matt.coster, maarten.lankhorst,
mripard, airlied, simona, dri-devel, linux-mm
Hi
Am 27.05.26 um 17:02 schrieb Igor Torrente:
> Hi Tomas,
>
> I tested your patch and Boris's modifications on top of your patch, and
> I'm happy to report that both seem to work just fine.
Thanks a lot for testing. I'll send out the official patch ASAP.
Best regards
Thomas
>
> BR,
> Igor Torrente
>
> On 5/27/26 07:32, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 27.05.26 um 12:18 schrieb Boris Brezillon:
>>> Hello Thomas,
>>>
>>> I'm inlining the diff you posted so I can comment on it directly before
>>> it's officially posted to the list.
>>
>> Thanks, I'll incorporate those in the next test rev or the official
>> patch.
>>
>> Best regards
>> Thomas
>>
>>>
>>> On Wed, 27 May 2026 08:56:33 +0200
>>> Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>
>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> index 545933c7f712..07e117673124 100644
>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> @@ -554,21 +554,6 @@ int drm_gem_shmem_dumb_create(struct drm_file
>>>> *file, struct drm_device *dev,
>>>> }
>>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
>>>> -static void drm_gem_shmem_record_mkwrite(struct vm_fault *vmf)
>>>> -{
>>>> - 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;
>>>> - pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page
>>>> offset within VMA */
>>>> -
>>>> - if (drm_WARN_ON(obj->dev, !shmem->pages || page_offset >=
>>>> num_pages))
>>>> - return;
>>>> -
>>>> - file_update_time(vma->vm_file);
>>>> - folio_mark_dirty(page_folio(shmem->pages[page_offset]));
>>>> -}
>>>> -
>>>> static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned
>>>> int order,
>>>> unsigned long pfn)
>>>> {
>>>> @@ -581,23 +566,16 @@ static vm_fault_t try_insert_pfn(struct
>>>> vm_fault *vmf, unsigned int order,
>>>> if (aligned &&
>>>> folio_test_pmd_mappable(page_folio(pfn_to_page(pfn)))) {
>>>> - vm_fault_t ret;
>>>> -
>>>> pfn &= PMD_MASK >> PAGE_SHIFT;
>>>> - /* Unlike PTEs which are automatically upgraded to
>>>> + /*
>>>> + * Unlike PTEs which are automatically upgraded to
>>>> * writeable entries, the PMD upgrades go through
>>>> * .huge_fault(). Make sure we pass the "write" info
>>>> * along in that case.
>>>> - * This also means we have to record the write fault
>>>> - * here, instead of in .pfn_mkwrite().
>>>> */
>>>> - ret = vmf_insert_pfn_pmd(vmf, pfn,
>>>> - vmf->flags & FAULT_FLAG_WRITE);
>>>> - if (ret == VM_FAULT_NOPAGE && (vmf->flags &
>>>> FAULT_FLAG_WRITE))
>>>> - drm_gem_shmem_record_mkwrite(vmf);
>>>> -
>>>> - return ret;
>>>> + return vmf_insert_pfn_pmd(vmf, pfn,
>>>> + vmf->flags & FAULT_FLAG_WRITE);
>>> I believe we can go back to
>>>
>>> return vmf_insert_pfn_pmd(vmf, pfn, false);
>>>
>>> if the mappings are no longer adjusted to catch write accesses.
>>>
>>>> }
>>>> #endif
>>>> }
>>>> @@ -635,8 +613,15 @@ static vm_fault_t
>>>> drm_gem_shmem_any_fault(struct vm_fault *vmf, unsigned int ord
>>>> pfn = page_to_pfn(page);
>>>> ret = try_insert_pfn(vmf, order, pfn);
>>>> - if (ret == VM_FAULT_NOPAGE)
>>>> + if (ret == VM_FAULT_NOPAGE) {
>>>> folio_mark_accessed(folio);
>>>> + /*
>>>> + * Always record write access to the buffer. The natural
>>>> + * place would be pfn_mkwrite, but this breaks KVM.
>>>> + */
>>>> + file_update_time(vma->vm_file);
>>>> + folio_mark_dirty(folio);
>>> We can be a bit smarter here:
>>>
>>> /*
>>> * Always record write access to the buffer if the
>>> * mapping is writeable. The natural place would be
>>> * pfn_mkwrite, but this breaks KVM.
>>> */
>>> if (vma->vm_flags & VM_WRITE) {
>>> file_update_time(vma->vm_file);
>>> folio_mark_dirty(folio);
>>> }
>>>
>>>> + }
>>> The rest looks good to me.
>>>
>>> Regards,
>>>
>>> Boris
>>
--
--
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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-27 10:18 ` Boris Brezillon
2026-05-27 10:32 ` Thomas Zimmermann
@ 2026-05-28 7:20 ` Thomas Zimmermann
2026-05-28 9:11 ` Boris Brezillon
1 sibling, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-05-28 7:20 UTC (permalink / raw)
To: Boris Brezillon
Cc: Igor Torrente, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona, dri-devel, linux-mm
Hi
Am 27.05.26 um 12:18 schrieb Boris Brezillon:
[...]
>> - return ret;
>> + return vmf_insert_pfn_pmd(vmf, pfn,
>> + vmf->flags & FAULT_FLAG_WRITE);
> I believe we can go back to
>
> return vmf_insert_pfn_pmd(vmf, pfn, false);
>
> if the mappings are no longer adjusted to catch write accesses.
If we don't install it as writable now, won't the kernel not split it up
into 4KiB pages when the actual write happens? If so, this might impact
performance negatively.
Best regards
Thomas
>
>> }
>> #endif
>> }
>> @@ -635,8 +613,15 @@ static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf, unsigned int ord
>> pfn = page_to_pfn(page);
>>
>> ret = try_insert_pfn(vmf, order, pfn);
>> - if (ret == VM_FAULT_NOPAGE)
>> + if (ret == VM_FAULT_NOPAGE) {
>> folio_mark_accessed(folio);
>> + /*
>> + * Always record write access to the buffer. The natural
>> + * place would be pfn_mkwrite, but this breaks KVM.
>> + */
>> + file_update_time(vma->vm_file);
>> + folio_mark_dirty(folio);
> We can be a bit smarter here:
>
> /*
> * Always record write access to the buffer if the
> * mapping is writeable. The natural place would be
> * pfn_mkwrite, but this breaks KVM.
> */
> if (vma->vm_flags & VM_WRITE) {
> file_update_time(vma->vm_file);
> folio_mark_dirty(folio);
> }
>
>> + }
> The rest looks good to me.
>
> Regards,
>
> Boris
--
--
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)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-28 7:20 ` Thomas Zimmermann
@ 2026-05-28 9:11 ` Boris Brezillon
2026-05-28 9:22 ` Thomas Zimmermann
0 siblings, 1 reply; 30+ messages in thread
From: Boris Brezillon @ 2026-05-28 9:11 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Igor Torrente, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona, dri-devel, linux-mm
On Thu, 28 May 2026 09:20:16 +0200
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 27.05.26 um 12:18 schrieb Boris Brezillon:
> [...]
> >> - return ret;
> >> + return vmf_insert_pfn_pmd(vmf, pfn,
> >> + vmf->flags & FAULT_FLAG_WRITE);
> > I believe we can go back to
> >
> > return vmf_insert_pfn_pmd(vmf, pfn, false);
> >
> > if the mappings are no longer adjusted to catch write accesses.
>
> If we don't install it as writable now, won't the kernel not split it up
> into 4KiB pages when the actual write happens?
It will be installed as writeable right away, regardless of the write
parameter, because if pfn_mkwrite is not implemented, vma->vm_page_prot
won't be lowered to read-only in the first place, or at least that's
what I remember from the previous debugging session I've done.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-28 9:11 ` Boris Brezillon
@ 2026-05-28 9:22 ` Thomas Zimmermann
2026-05-28 9:38 ` Boris Brezillon
0 siblings, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-05-28 9:22 UTC (permalink / raw)
To: Boris Brezillon
Cc: Igor Torrente, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona, dri-devel, linux-mm
Hi
Am 28.05.26 um 11:11 schrieb Boris Brezillon:
> On Thu, 28 May 2026 09:20:16 +0200
> Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>> Hi
>>
>> Am 27.05.26 um 12:18 schrieb Boris Brezillon:
>> [...]
>>>> - return ret;
>>>> + return vmf_insert_pfn_pmd(vmf, pfn,
>>>> + vmf->flags & FAULT_FLAG_WRITE);
>>> I believe we can go back to
>>>
>>> return vmf_insert_pfn_pmd(vmf, pfn, false);
>>>
>>> if the mappings are no longer adjusted to catch write accesses.
>> If we don't install it as writable now, won't the kernel not split it up
>> into 4KiB pages when the actual write happens?
> It will be installed as writeable right away, regardless of the write
> parameter, because if pfn_mkwrite is not implemented, vma->vm_page_prot
> won't be lowered to read-only in the first place, or at least that's
> what I remember from the previous debugging session I've done.
In insert_pmd(), the write parameter controls whether we run
maybe_pmd_mkwrite(), which sets the __PAGE_RW flag.
https://elixir.bootlin.com/linux/v7.1-rc5/source/mm/huge_memory.c#L1632
https://elixir.bootlin.com/linux/v7.1-rc5/source/mm/huge_memory.c#L1655
That's at least how I understand the logic here. Performance-wise it
might be beneficial if we track the write flags as before.
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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap
2026-05-28 9:22 ` Thomas Zimmermann
@ 2026-05-28 9:38 ` Boris Brezillon
0 siblings, 0 replies; 30+ messages in thread
From: Boris Brezillon @ 2026-05-28 9:38 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Igor Torrente, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona, dri-devel, linux-mm
On Thu, 28 May 2026 11:22:46 +0200
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 28.05.26 um 11:11 schrieb Boris Brezillon:
> > On Thu, 28 May 2026 09:20:16 +0200
> > Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> >> Hi
> >>
> >> Am 27.05.26 um 12:18 schrieb Boris Brezillon:
> >> [...]
> >>>> - return ret;
> >>>> + return vmf_insert_pfn_pmd(vmf, pfn,
> >>>> + vmf->flags & FAULT_FLAG_WRITE);
> >>> I believe we can go back to
> >>>
> >>> return vmf_insert_pfn_pmd(vmf, pfn, false);
> >>>
> >>> if the mappings are no longer adjusted to catch write accesses.
> >> If we don't install it as writable now, won't the kernel not split it up
> >> into 4KiB pages when the actual write happens?
> > It will be installed as writeable right away, regardless of the write
> > parameter, because if pfn_mkwrite is not implemented, vma->vm_page_prot
> > won't be lowered to read-only in the first place, or at least that's
> > what I remember from the previous debugging session I've done.
>
> In insert_pmd(), the write parameter controls whether we run
> maybe_pmd_mkwrite(), which sets the __PAGE_RW flag.
>
> https://elixir.bootlin.com/linux/v7.1-rc5/source/mm/huge_memory.c#L1632
> https://elixir.bootlin.com/linux/v7.1-rc5/source/mm/huge_memory.c#L1655
This is not where the RW|SHARED -> READ_ONLY lowering happens though. If
the write argument was the only way to get writeable hugepage mappings,
it wouldn't have worked before your pfn_mkwrite addition.
>
> That's at least how I understand the logic here. Performance-wise it
> might be beneficial if we track the write flags as before.
It was working fine before pfn_mkwrite was added: huge pages were
mapped as PMDs even when they were writable.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 6/6] drm/gem-shmem: Track folio accessed/dirty status in vmap
2026-02-09 13:27 [PATCH v3 0/6] drm/gem-shmem: Track page accessed/dirty status Thomas Zimmermann
` (4 preceding siblings ...)
2026-02-09 13:27 ` [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap Thomas Zimmermann
@ 2026-02-09 13:27 ` Thomas Zimmermann
2026-02-25 10:57 ` Frank Binns
5 siblings, 1 reply; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-09 13:27 UTC (permalink / raw)
To: boris.brezillon, loic.molinari, willy, frank.binns, matt.coster,
maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, linux-mm, Thomas Zimmermann
On successful vmap, set the page_mark_accessed_on_put and _dirty_on_put
flags in the gem-shmem object. Signals that the contained pages require
LRU and dirty tracking when they are being released back to SHMEM. Clear
these flags on put, so that the buffer remains quiet until the next call
to vmap. There's no means of handling dirty status in vmap as there's no
write-only mapping available.
Both flags, _accessed_on_put and _dirty_on_put, have always been part of
the gem-shmem object, but never used much. So most drivers did not track
the page status correctly.
Only the v3d and imagination drivers make limited use of _dirty_on_put. In
the case of imagination, move the flag setting from init to cleanup. This
ensures writeback of modified pages but does not interfere with the
internal vmap/vunmap calls. V3d already implements this behaviour.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> # gem-shmem
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++++
drivers/gpu/drm/imagination/pvr_gem.c | 6 ++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0c86ad40a049..dda9af9bf5b3 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -265,6 +265,8 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
shmem->pages_mark_dirty_on_put,
shmem->pages_mark_accessed_on_put);
shmem->pages = NULL;
+ shmem->pages_mark_accessed_on_put = false;
+ shmem->pages_mark_dirty_on_put = false;
}
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked);
@@ -397,6 +399,8 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
} else {
iosys_map_set_vaddr(map, shmem->vaddr);
refcount_set(&shmem->vmap_use_count, 1);
+ shmem->pages_mark_accessed_on_put = true;
+ shmem->pages_mark_dirty_on_put = true;
}
}
diff --git a/drivers/gpu/drm/imagination/pvr_gem.c b/drivers/gpu/drm/imagination/pvr_gem.c
index c07c9a915190..307b02c916d4 100644
--- a/drivers/gpu/drm/imagination/pvr_gem.c
+++ b/drivers/gpu/drm/imagination/pvr_gem.c
@@ -25,7 +25,10 @@
static void pvr_gem_object_free(struct drm_gem_object *obj)
{
- drm_gem_shmem_object_free(obj);
+ struct drm_gem_shmem_object *shmem_obj = to_drm_gem_shmem_obj(obj);
+
+ shmem_obj->pages_mark_dirty_on_put = true;
+ drm_gem_shmem_free(shmem_obj);
}
static struct dma_buf *pvr_gem_export(struct drm_gem_object *obj, int flags)
@@ -363,7 +366,6 @@ pvr_gem_object_create(struct pvr_device *pvr_dev, size_t size, u64 flags)
if (IS_ERR(shmem_obj))
return ERR_CAST(shmem_obj);
- shmem_obj->pages_mark_dirty_on_put = true;
shmem_obj->map_wc = !(flags & PVR_BO_CPU_CACHED);
pvr_obj = shmem_gem_to_pvr_gem(shmem_obj);
pvr_obj->flags = flags;
--
2.52.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 6/6] drm/gem-shmem: Track folio accessed/dirty status in vmap
2026-02-09 13:27 ` [PATCH v3 6/6] drm/gem-shmem: Track folio accessed/dirty status in vmap Thomas Zimmermann
@ 2026-02-25 10:57 ` Frank Binns
2026-02-25 11:34 ` Thomas Zimmermann
0 siblings, 1 reply; 30+ messages in thread
From: Frank Binns @ 2026-02-25 10:57 UTC (permalink / raw)
To: tzimmermann@suse.de, Matt Coster, simona@ffwll.ch,
airlied@gmail.com, boris.brezillon@collabora.com,
loic.molinari@collabora.com, willy@infradead.org,
mripard@kernel.org, maarten.lankhorst@linux.intel.com
Cc: dri-devel@lists.freedesktop.org, linux-mm@kvack.org
Hi Thomas,
On Mon, 2026-02-09 at 14:27 +0100, Thomas Zimmermann wrote:
> On successful vmap, set the page_mark_accessed_on_put and _dirty_on_put
> flags in the gem-shmem object. Signals that the contained pages require
> LRU and dirty tracking when they are being released back to SHMEM. Clear
> these flags on put, so that the buffer remains quiet until the next call
> to vmap. There's no means of handling dirty status in vmap as there's no
> write-only mapping available.
>
> Both flags, _accessed_on_put and _dirty_on_put, have always been part of
> the gem-shmem object, but never used much. So most drivers did not track
> the page status correctly.
>
> Only the v3d and imagination drivers make limited use of _dirty_on_put. In
> the case of imagination, move the flag setting from init to cleanup. This
> ensures writeback of modified pages but does not interfere with the
> internal vmap/vunmap calls. V3d already implements this behaviour.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> # gem-shmem
The change to the imagination driver is:
Acked-by: Frank Binns <frank.binns@imgtec.com>
Tested-by: Frank Binns <frank.binns@imgtec.com>
Thanks
Frank
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++++
> drivers/gpu/drm/imagination/pvr_gem.c | 6 ++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 0c86ad40a049..dda9af9bf5b3 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -265,6 +265,8 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
> shmem->pages_mark_dirty_on_put,
> shmem->pages_mark_accessed_on_put);
> shmem->pages = NULL;
> + shmem->pages_mark_accessed_on_put = false;
> + shmem->pages_mark_dirty_on_put = false;
> }
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked);
> @@ -397,6 +399,8 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
> } else {
> iosys_map_set_vaddr(map, shmem->vaddr);
> refcount_set(&shmem->vmap_use_count, 1);
> + shmem->pages_mark_accessed_on_put = true;
> + shmem->pages_mark_dirty_on_put = true;
> }
> }
>
> diff --git a/drivers/gpu/drm/imagination/pvr_gem.c b/drivers/gpu/drm/imagination/pvr_gem.c
> index c07c9a915190..307b02c916d4 100644
> --- a/drivers/gpu/drm/imagination/pvr_gem.c
> +++ b/drivers/gpu/drm/imagination/pvr_gem.c
> @@ -25,7 +25,10 @@
>
> static void pvr_gem_object_free(struct drm_gem_object *obj)
> {
> - drm_gem_shmem_object_free(obj);
> + struct drm_gem_shmem_object *shmem_obj = to_drm_gem_shmem_obj(obj);
> +
> + shmem_obj->pages_mark_dirty_on_put = true;
> + drm_gem_shmem_free(shmem_obj);
> }
>
> static struct dma_buf *pvr_gem_export(struct drm_gem_object *obj, int flags)
> @@ -363,7 +366,6 @@ pvr_gem_object_create(struct pvr_device *pvr_dev, size_t size, u64 flags)
> if (IS_ERR(shmem_obj))
> return ERR_CAST(shmem_obj);
>
> - shmem_obj->pages_mark_dirty_on_put = true;
> shmem_obj->map_wc = !(flags & PVR_BO_CPU_CACHED);
> pvr_obj = shmem_gem_to_pvr_gem(shmem_obj);
> pvr_obj->flags = flags;
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 6/6] drm/gem-shmem: Track folio accessed/dirty status in vmap
2026-02-25 10:57 ` Frank Binns
@ 2026-02-25 11:34 ` Thomas Zimmermann
0 siblings, 0 replies; 30+ messages in thread
From: Thomas Zimmermann @ 2026-02-25 11:34 UTC (permalink / raw)
To: Frank Binns, Matt Coster, simona@ffwll.ch, airlied@gmail.com,
boris.brezillon@collabora.com, loic.molinari@collabora.com,
willy@infradead.org, mripard@kernel.org,
maarten.lankhorst@linux.intel.com
Cc: dri-devel@lists.freedesktop.org, linux-mm@kvack.org
Am 25.02.26 um 11:57 schrieb Frank Binns:
> Hi Thomas,
>
> On Mon, 2026-02-09 at 14:27 +0100, Thomas Zimmermann wrote:
>> On successful vmap, set the page_mark_accessed_on_put and _dirty_on_put
>> flags in the gem-shmem object. Signals that the contained pages require
>> LRU and dirty tracking when they are being released back to SHMEM. Clear
>> these flags on put, so that the buffer remains quiet until the next call
>> to vmap. There's no means of handling dirty status in vmap as there's no
>> write-only mapping available.
>>
>> Both flags, _accessed_on_put and _dirty_on_put, have always been part of
>> the gem-shmem object, but never used much. So most drivers did not track
>> the page status correctly.
>>
>> Only the v3d and imagination drivers make limited use of _dirty_on_put. In
>> the case of imagination, move the flag setting from init to cleanup. This
>> ensures writeback of modified pages but does not interfere with the
>> internal vmap/vunmap calls. V3d already implements this behaviour.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> # gem-shmem
> The change to the imagination driver is:
>
> Acked-by: Frank Binns <frank.binns@imgtec.com>
> Tested-by: Frank Binns <frank.binns@imgtec.com>
Great, thanks a lot.
>
> Thanks
> Frank
>
>> ---
>> drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++++
>> drivers/gpu/drm/imagination/pvr_gem.c | 6 ++++--
>> 2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index 0c86ad40a049..dda9af9bf5b3 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -265,6 +265,8 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
>> shmem->pages_mark_dirty_on_put,
>> shmem->pages_mark_accessed_on_put);
>> shmem->pages = NULL;
>> + shmem->pages_mark_accessed_on_put = false;
>> + shmem->pages_mark_dirty_on_put = false;
>> }
>> }
>> EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked);
>> @@ -397,6 +399,8 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
>> } else {
>> iosys_map_set_vaddr(map, shmem->vaddr);
>> refcount_set(&shmem->vmap_use_count, 1);
>> + shmem->pages_mark_accessed_on_put = true;
>> + shmem->pages_mark_dirty_on_put = true;
>> }
>> }
>>
>> diff --git a/drivers/gpu/drm/imagination/pvr_gem.c b/drivers/gpu/drm/imagination/pvr_gem.c
>> index c07c9a915190..307b02c916d4 100644
>> --- a/drivers/gpu/drm/imagination/pvr_gem.c
>> +++ b/drivers/gpu/drm/imagination/pvr_gem.c
>> @@ -25,7 +25,10 @@
>>
>> static void pvr_gem_object_free(struct drm_gem_object *obj)
>> {
>> - drm_gem_shmem_object_free(obj);
>> + struct drm_gem_shmem_object *shmem_obj = to_drm_gem_shmem_obj(obj);
>> +
>> + shmem_obj->pages_mark_dirty_on_put = true;
>> + drm_gem_shmem_free(shmem_obj);
>> }
>>
>> static struct dma_buf *pvr_gem_export(struct drm_gem_object *obj, int flags)
>> @@ -363,7 +366,6 @@ pvr_gem_object_create(struct pvr_device *pvr_dev, size_t size, u64 flags)
>> if (IS_ERR(shmem_obj))
>> return ERR_CAST(shmem_obj);
>>
>> - shmem_obj->pages_mark_dirty_on_put = true;
>> shmem_obj->map_wc = !(flags & PVR_BO_CPU_CACHED);
>> pvr_obj = shmem_gem_to_pvr_gem(shmem_obj);
>> pvr_obj->flags = flags;
--
--
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)
^ permalink raw reply [flat|nested] 30+ messages in thread