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 <[email protected]> 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

Reply via email to