On Thu, Jul 16, 2026 at 11:43:35AM +0200, Vlastimil Babka (SUSE) wrote:
> On 7/16/26 11:27, Lorenzo Stoakes (ARM) wrote:
> > On Wed, Jul 15, 2026 at 05:23:09PM +0200, Vlastimil Babka (SUSE) wrote:
> >> On 7/10/26 22:16, Lorenzo Stoakes wrote:
> >> > There are many instances in which linear_page_index() (as well as
> >> > linear_page_delta()) is open-coded, which is confusing and inconsistent.
> >> >
> >> > Additionally, vma->vm_pgoff doesn't necessarily make it clear that this
> >> > is
> >> > the page offset of the start of the VMA range.
> >> >
> >> > Doing so also aids greppability.
> >> >
> >> > So use vma_start_pgoff() in favour of directly accessing vma->vm_pgoff,
> >> > and
> >> > linear_page_index() where we can.
> >> >
> >> > This also lays the ground for future changes which will add an anonymous
> >> > page offset in order to be able to index MAP_PRIVATE-file backed anon
> >> > folios in terms of their virtual page offset.
> >> >
> >> > No functional change intended.
> >> >
> >> > Reviewed-by: Gregory Price <[email protected]>
> >> > Reviewed-by: SJ Park <[email protected]>
> >> > Reviewed-by: Pedro Falcato <[email protected]>
> >> > Signed-off-by: Lorenzo Stoakes <[email protected]>
> >>
> >> Reviewed-by: Vlastimil Babka (SUSE) <[email protected]>
> >
> > Thanks!
> >
> >>
> >> Nit:
> >>
> >> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> >> > index 2058db9c01d5..d10b8042adb5 100644
> >> > --- a/mm/damon/vaddr.c
> >> > +++ b/mm/damon/vaddr.c
> >> > @@ -10,6 +10,7 @@
> >> > #include <linux/mman.h>
> >> > #include <linux/mmu_notifier.h>
> >> > #include <linux/page_idle.h>
> >> > +#include <linux/pagemap.h>
> >> > #include <linux/pagewalk.h>
> >> > #include <linux/sched/mm.h>
> >> >
> >> > @@ -623,8 +624,8 @@ static void damos_va_migrate_dests_add(struct folio
> >> > *folio,
> >> > }
> >> >
> >> > order = folio_order(folio);
> >> > - ilx = vma->vm_pgoff >> order;
> >> > - ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> >> > + ilx = vma_start_pgoff(vma) >> order;
> >> > + ilx += linear_page_delta(vma, addr) >> order;
> >>
> >> Could these be combined to linear_page_index(vma, addr) >> order?
> >
> > So this is:
> >
> > (x >> order) + (y >> order)
> >
> > And linear_page_index(vma, addr) >> order is:
> >
> > (x + y) >> order
> >
> > Consider:
> >
> > order = 1, x = 0b11, y = 0b01
> >
> > (x >> order) + (y >> order) =
> > (0b11 >> 1 ) + (0b01 >> 1 ) =
> > 0b1 + 0 =
> > 1
> >
> > (x + y ) >> order =
> > (0b11 + 0b01) >> 1 =
> > (0b100 ) >> 1 =
> > 2
> >
> > They are not equivalent - the carry bit changes the result.
>
> Right, thanks. Didn't consider it would be actually working with unaligned
> addresses but seems like it can.
Yeah, I initially did it the way you suggested then ran it by AI review and it
moaned at me so ;)
>
> >>
> >> >
> >> > for (i = 0; i < dests->nr_dests; i++)
> >> > weight_total += dests->weight_arr[i];
> >>
> >> > diff --git a/mm/filemap.c b/mm/filemap.c
> >> > index b39111abdc4b..1dbb4c6f824e 100644
> >> > --- a/mm/filemap.c
> >> > +++ b/mm/filemap.c
> >> > @@ -3411,8 +3411,8 @@ static struct file *do_sync_mmap_readahead(struct
> >> > vm_fault *vmf)
> >> > * of memory.
> >> > */
> >> > struct vm_area_struct *vma = vmf->vma;
> >> > - unsigned long start = vma->vm_pgoff;
> >> > - unsigned long end = start + vma_pages(vma);
> >> > + const unsigned long start = vma_start_pgoff(vma);
> >> > + const unsigned long end = vma_end_pgoff(vma);
> >> > unsigned long ra_end;
> >> >
> >> > ra->order = exec_folio_order();
> >> > @@ -3930,7 +3930,8 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf,
> >> > goto out;
> >> > }
> >> >
> >> > - addr = vma->vm_start + ((start_pgoff - vma->vm_pgoff) <<
> >> > PAGE_SHIFT);
> >> > + addr = vma->vm_start +
> >> > + ((start_pgoff - vma_start_pgoff(vma)) << PAGE_SHIFT);
> >>
> >> This could be linear_page_index(vma, start_pgoff) <<
>
> Thanks for pretending you didn't notice this one, which was even more
> obviously incorrect :)
Again I screwed this up when I first did it so was far more terribly wrong
myself :P All hail AI review I suppose?
>
> >> > vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, addr,
> >> > &vmf->ptl);
> >> > if (!vmf->pte) {
> >> > folio_unlock(folio);
> >>
> >> > --- a/mm/mempolicy.c
> >> > +++ b/mm/mempolicy.c
> >> > @@ -2049,8 +2049,8 @@ struct mempolicy *get_vma_policy(struct
> >> > vm_area_struct *vma,
> >> > pol = get_task_policy(current);
> >> > if (pol->mode == MPOL_INTERLEAVE ||
> >> > pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> >> > - *ilx += vma->vm_pgoff >> order;
> >> > - *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> >> > + *ilx += vma_start_pgoff(vma) >> order;
> >> > + *ilx += linear_page_delta(vma, addr) >> order;
> >>
> >> Also?
> >
> > See above.
> >
> >>
> >> > }
> >> > return pol;
> >> > }
> >>
> >> > --- a/mm/nommu.c
> >> > +++ b/mm/nommu.c
> >> > @@ -975,7 +975,7 @@ static int do_mmap_private(struct vm_area_struct
> >> > *vma,
> >> > /* read the contents of a file into the copy */
> >> > loff_t fpos;
> >> >
> >> > - fpos = vma->vm_pgoff;
> >> > + fpos = vma_start_pgoff(vma);
> >> > fpos <<= PAGE_SHIFT;
> >> >
> >> > ret = kernel_read(vma->vm_file, base, len, &fpos);
> >> > @@ -1378,7 +1378,8 @@ static int split_vma(struct vma_iterator *vmi,
> >> > struct vm_area_struct *vma,
> >> > delete_nommu_region(vma->vm_region);
> >> > if (new_below) {
> >> > vma->vm_region->vm_start = vma->vm_start = addr;
> >> > - vma->vm_region->vm_pgoff = vma->vm_pgoff += npages;
> >>
> >> yuck, good to remove that ugly construct
> >> it's not the only occurence in this function
> >> oh well
> >
> > Yup at least improved it here :)
>
> And a later patch removes the other occurence.
A game of 2 halves!
>
> >>
> >> > + vma->vm_pgoff += npages;
> >> > + vma->vm_region->vm_pgoff = vma_start_pgoff(vma);
> >> > } else {
> >> > vma->vm_region->vm_end = vma->vm_end = addr;
> >> > vma->vm_region->vm_top = addr;
> >
> > Cheers, Lorenzo
>
