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

[Boris Brezillon <boris.brezillon@collabora.com>]

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