Re: [PATCH v3 5/6] drm/gem-shmem: Track folio accessed/dirty status in mmap

[Igor Torrente <igor.torrente@collabora.com>]

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);
>