Hi Boris,
On 01/12/2025 09:05, Boris Brezillon wrote:
> On Fri, 28 Nov 2025 19:52:43 +0100
> Loïc Molinari <loic.molinari@collabora.com> wrote:
>
>> For a fault at address addr, the page offset is
>> page_offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT
>> = ((addr & PAGE_MASK) - vma->vm_start) >> PAGE_SHIFT
>> = (addr - vma->vm_start) >> PAGE_SHIFT
>>
>> Since the faulty logical page offset based on VMA is
>> vmf->pgoff = vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT)
>>
>> We can slightly simplify the calculation using
>> page_offset = vmf->pgoff - vma->vm_pgoff
>>
>> Signed-off-by: Loïc Molinari <loic.molinari@collabora.com>
>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
>
> One nit below
>
>> ---
>> drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index dc94a27710e5..be89be1c804c 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -577,8 +577,8 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
>> struct page *page;
>> pgoff_t page_offset;
>>
>> - /* We don't use vmf->pgoff since that has the fake offset */
>> - page_offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
>> + /* Offset to faulty address in the VMA (without the fake offset). */
>
> It's weird to say "without the fake offset" here, because IIUC, both
> vmf->pgoff and vma->vm_pgoff contain the fake offset. And that's fine,
> the problem really is when one of the subtraction operand is not
> relative to the fake offset.
Yes, both values contain the fake offset. vma->vm_pgoff is the actual
fake offset (mmap offset in the GEM context). vmf->pgoff is the fake
offset added to the offset we're looking for (offset from start of VMA
to faulty address). So the difference just gets rid of it, hence the
precision, but now that I read it again after a few weeks, it's a bit
misleading so I'll just remove it.
Regards,
Loïc
>> + page_offset = vmf->pgoff - vma->vm_pgoff;
>>
>> dma_resv_lock(shmem->base.resv, NULL);
>>
>