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

[Thomas Zimmermann <tzimmermann@suse.de>]

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)