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 <[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