Re: [PATCH v10 01/10] drm/shmem-helper: Simplify page offset calculation in fault handler

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

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.

> +	page_offset = vmf->pgoff - vma->vm_pgoff;
>  
>  	dma_resv_lock(shmem->base.resv, NULL);
>