On Thu, Jul 02, 2026 at 12:01:37PM +0100, Pedro Falcato wrote:
> On Mon, Jun 29, 2026 at 01:23:28PM +0100, Lorenzo Stoakes wrote:
> > Be consistent in using vma_start_pgoff() and vma_end_pgoff(), which clearly
> > indicates which part of the VMA the page offset refers to and aids
> > greppability.
> >
> > This is part of a broader series laying the ground to provide a virtual
> > page offset for MAP_PRIVATE-file backed anon folios.
> >
> > No functional change intended.
> >
> > Signed-off-by: Lorenzo Stoakes <[email protected]>
> > ---
> > kernel/dma/coherent.c | 7 ++++---
> > kernel/dma/direct.c | 6 ++++--
> > kernel/dma/mapping.c | 8 +++++---
> > kernel/dma/ops_helpers.c | 4 ++--
> > kernel/events/core.c | 20 +++++++++++---------
> > kernel/events/uprobes.c | 11 +++++++----
> > kernel/kcov.c | 2 +-
> > kernel/trace/ring_buffer.c | 3 ++-
> > 8 files changed, 36 insertions(+), 25 deletions(-)
> >
> > diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> > index bcdc0f76d2e8..2d3195eb7e83 100644
> > --- a/kernel/dma/coherent.c
> > +++ b/kernel/dma/coherent.c
> > @@ -236,14 +236,15 @@ static int __dma_mmap_from_coherent(struct
> > dma_coherent_mem *mem,
> > {
> > if (mem && vaddr >= mem->virt_base && vaddr + size <=
> > (mem->virt_base + ((dma_addr_t)mem->size << PAGE_SHIFT))) {
> > - unsigned long off = vma->vm_pgoff;
> > + const pgoff_t pgoff_start = vma_start_pgoff(vma);
> > + const pgoff_t pgoff_end = vma_end_pgoff(vma);
> > int start = (vaddr - mem->virt_base) >> PAGE_SHIFT;
> > unsigned long user_count = vma_pages(vma);
> > int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >
> > *ret = -ENXIO;
> > - if (off < count && user_count <= count - off) {
> > - unsigned long pfn = mem->pfn_base + start + off;
> > + if (pgoff_start < count && pgoff_end <= count) {
> > + unsigned long pfn = mem->pfn_base + start + pgoff_start;
> > *ret = remap_pfn_range(vma, vma->vm_start, pfn,
> > user_count << PAGE_SHIFT,
> > vma->vm_page_prot);
> > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> > index 4391b797d4db..436310d6e4a2 100644
> > --- a/kernel/dma/direct.c
> > +++ b/kernel/dma/direct.c
> > @@ -534,6 +534,8 @@ int dma_direct_mmap(struct device *dev, struct
> > vm_area_struct *vma,
> > unsigned long user_count = vma_pages(vma);
> > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> > unsigned long pfn = PHYS_PFN(dma_to_phys(dev, dma_addr));
> > + const pgoff_t pgoff_start = vma_start_pgoff(vma);
> > + const pgoff_t pgoff_end = vma_end_pgoff(vma);
> > int ret = -ENXIO;
> >
> > vma->vm_page_prot = dma_pgprot(dev, vma->vm_page_prot, attrs);
> > @@ -545,9 +547,9 @@ int dma_direct_mmap(struct device *dev, struct
> > vm_area_struct *vma,
> > if (dma_mmap_from_global_coherent(vma, cpu_addr, size, &ret))
> > return ret;
> >
> > - if (vma->vm_pgoff >= count || user_count > count - vma->vm_pgoff)
> > + if (pgoff_start >= count || pgoff_end > count)
> > return -ENXIO;
> > - return remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff,
> > + return remap_pfn_range(vma, vma->vm_start, pfn + pgoff_start,
> > user_count << PAGE_SHIFT, vma->vm_page_prot);
> > }
> >
> > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> > index 4fe04669e5e6..c986639044e9 100644
> > --- a/kernel/dma/mapping.c
> > +++ b/kernel/dma/mapping.c
> > @@ -761,12 +761,14 @@ EXPORT_SYMBOL_GPL(dma_free_pages);
> > int dma_mmap_pages(struct device *dev, struct vm_area_struct *vma,
> > size_t size, struct page *page)
> > {
> > - unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> > + const pgoff_t pgoff_start = vma_start_pgoff(vma);
> > + const pgoff_t pgoff_end = vma_end_pgoff(vma);
> > + const unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >
> > - if (vma->vm_pgoff >= count || vma_pages(vma) > count - vma->vm_pgoff)
> > + if (pgoff_start >= count || pgoff_end > count)
> > return -ENXIO;
> > return remap_pfn_range(vma, vma->vm_start,
> > - page_to_pfn(page) + vma->vm_pgoff,
> > + page_to_pfn(page) + pgoff_start,
> > vma_pages(vma) << PAGE_SHIFT, vma->vm_page_prot);
> > }
> > EXPORT_SYMBOL_GPL(dma_mmap_pages);
> > diff --git a/kernel/dma/ops_helpers.c b/kernel/dma/ops_helpers.c
> > index 20caf9cabf69..6b5f9208d31c 100644
> > --- a/kernel/dma/ops_helpers.c
> > +++ b/kernel/dma/ops_helpers.c
> > @@ -39,7 +39,7 @@ int dma_common_mmap(struct device *dev, struct
> > vm_area_struct *vma,
> > #ifdef CONFIG_MMU
> > unsigned long user_count = vma_pages(vma);
> > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> > - unsigned long off = vma->vm_pgoff;
> > + unsigned long off = vma_start_pgoff(vma);
> > struct page *page = dma_common_vaddr_to_page(cpu_addr);
> > int ret = -ENXIO;
> >
> > @@ -52,7 +52,7 @@ int dma_common_mmap(struct device *dev, struct
> > vm_area_struct *vma,
> > return -ENXIO;
> >
> > return remap_pfn_range(vma, vma->vm_start,
> > - page_to_pfn(page) + vma->vm_pgoff,
> > + page_to_pfn(page) + vma_start_pgoff(vma),
> > user_count << PAGE_SHIFT, vma->vm_page_prot);
> > #else
> > return -ENXIO;
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 954c36e28101..d6d2d557ccb8 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -6998,7 +6998,7 @@ static void perf_mmap_open(struct vm_area_struct *vma)
> > refcount_inc(&event->mmap_count);
> > refcount_inc(&event->rb->mmap_count);
> >
> > - if (vma->vm_pgoff)
> > + if (vma_start_pgoff(vma))
> > refcount_inc(&event->rb->aux_mmap_count);
> >
> > if (mapped)
> > @@ -7032,7 +7032,7 @@ static void perf_mmap_close(struct vm_area_struct
> > *vma)
> > * The AUX buffer is strictly a sub-buffer, serialize using aux_mutex
> > * to avoid complications.
> > */
> > - if (rb_has_aux(rb) && vma->vm_pgoff == rb->aux_pgoff &&
> > + if (rb_has_aux(rb) && vma_start_pgoff(vma) == rb->aux_pgoff &&
> > refcount_dec_and_mutex_lock(&rb->aux_mmap_count, &rb->aux_mutex)) {
> > /*
> > * Stop all AUX events that are writing to this buffer,
> > @@ -7190,7 +7190,8 @@ static int map_range(struct perf_buffer *rb, struct
> > vm_area_struct *vma)
> > */
> > for (pagenum = 0; pagenum < nr_pages; pagenum++) {
> > unsigned long va = vma->vm_start + PAGE_SIZE * pagenum;
> > - struct page *page = perf_mmap_to_page(rb, vma->vm_pgoff +
> > pagenum);
> > + struct page *page = perf_mmap_to_page(rb,
> > + vma_start_pgoff(vma) + pagenum);
> >
> > if (page == NULL) {
> > err = -EINVAL;
> > @@ -7348,6 +7349,7 @@ static int perf_mmap_aux(struct vm_area_struct *vma,
> > struct perf_event *event,
> > u64 aux_offset, aux_size;
> > struct perf_buffer *rb;
> > int ret, rb_flags = 0;
> > + const pgoff_t pgoff_start = vma_start_pgoff(vma);
>
> Variable decs here seem to be in reverse christmas tree order, so perhaps
> move this to the top.
Ack will change on respin.
>
> >
> > rb = event->rb;
> > if (!rb)
> > @@ -7366,11 +7368,11 @@ static int perf_mmap_aux(struct vm_area_struct
> > *vma, struct perf_event *event,
> > if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
> > return -EINVAL;
> >
> > - if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
> > + if (aux_offset != pgoff_start << PAGE_SHIFT)
> > return -EINVAL;
> >
> > /* already mapped with a different offset */
> > - if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
> > + if (rb_has_aux(rb) && rb->aux_pgoff != pgoff_start)
> > return -EINVAL;
> >
> > if (aux_size != nr_pages * PAGE_SIZE)
> > @@ -7400,7 +7402,7 @@ static int perf_mmap_aux(struct vm_area_struct *vma,
> > struct perf_event *event,
> > if (vma->vm_flags & VM_WRITE)
> > rb_flags |= RING_BUFFER_WRITABLE;
> >
> > - ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> > + ret = rb_alloc_aux(rb, event, pgoff_start, nr_pages,
> > event->attr.aux_watermark, rb_flags);
> > if (ret) {
> > refcount_dec(&rb->mmap_count);
> > @@ -7457,7 +7459,7 @@ static int perf_mmap(struct file *file, struct
> > vm_area_struct *vma)
> > if (event->state <= PERF_EVENT_STATE_REVOKED)
> > return -ENODEV;
> >
> > - if (vma->vm_pgoff == 0)
> > + if (!vma_start_pgoff(vma))
> > ret = perf_mmap_rb(vma, event, nr_pages);
> > else
> > ret = perf_mmap_aux(vma, event, nr_pages);
> > @@ -9884,7 +9886,7 @@ static bool perf_addr_filter_vma_adjust(struct
> > perf_addr_filter *filter,
> > struct perf_addr_filter_range *fr)
> > {
> > unsigned long vma_size = vma->vm_end - vma->vm_start;
> > - unsigned long off = vma->vm_pgoff << PAGE_SHIFT;
> > + unsigned long off = vma_start_pgoff(vma) << PAGE_SHIFT;
> > struct file *file = vma->vm_file;
> >
> > if (!perf_addr_filter_match(filter, file, off, vma_size))
> > @@ -9974,7 +9976,7 @@ void perf_event_mmap(struct vm_area_struct *vma)
> > /* .tid */
> > .start = vma->vm_start,
> > .len = vma->vm_end - vma->vm_start,
> > - .pgoff = (u64)vma->vm_pgoff << PAGE_SHIFT,
> > + .pgoff = (u64)vma_start_pgoff(vma) << PAGE_SHIFT,
> > },
> > /* .maj (attr_mmap2 only) */
> > /* .min (attr_mmap2 only) */
> > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> > index f23cebacbc6d..244651380ca1 100644
> > --- a/kernel/events/uprobes.c
> > +++ b/kernel/events/uprobes.c
> > @@ -144,12 +144,14 @@ static bool valid_vma(struct vm_area_struct *vma,
> > bool is_register)
> >
> > static unsigned long offset_to_vaddr(struct vm_area_struct *vma, loff_t
> > offset)
> > {
> > - return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
> > + return vma->vm_start + offset -
> > + ((loff_t)vma_start_pgoff(vma) << PAGE_SHIFT);
> > }
> >
> > static loff_t vaddr_to_offset(struct vm_area_struct *vma, unsigned long
> > vaddr)
> > {
> > - return ((loff_t)vma->vm_pgoff << PAGE_SHIFT) + (vaddr - vma->vm_start);
> > + return ((loff_t)vma_start_pgoff(vma) << PAGE_SHIFT) +
> > + (vaddr - vma->vm_start);
> > }
>
> Something we've seen in this series is that perhaps something like
>
> static inline loff_t vma_start_off(vma)
> {
> return ((loff_t) vma_start_pgoff(vma)) << PAGE_SHIFT;
> }
>
> could be worth it.
Yeah I already thought about this kind of thing, but in the end decided against
it at least for now, maybe I could revisit with something like that added
though...
But given this series is being held off on already for acceptance, I'm not sure
adding _yet more_ changes will be welcomed.
Plus I worry people will get confused.
There's 2 forms I noticed:
1. effectively: linear_page_index(vma, address) << PAGE_SHIFT
I wanted to rewrite this as such, but you can't, because of course vaddr might
have non-page aligned bits (or tags) that you lose by doing that. So ugh.
You'd then need to write a slightly nuanced version like:
static inline unsigned long vma_offset(const struct vm_area_struct *vma,
const unsigned long address)
{
/* Retains page offset and tags. */
return address - vma->vm_start;
}
static inline unsigned long linear_page_offset(const struct vm_area_struct *vma,
const unsigned long address)
{
const unsigned long addr = vma_start_pgoff(vma) << PAGE_SHIFT;
addr += linear_delta(vma, address);
return addr;
}
(Could also do:)
static inline pgoff_t linear_page_delta(const struct vm_area_struct *vma,
const unsigned long address)
{
return vma_offset(vma, address) >> PAGE_SHIFT;
}
BUT.
I think this will confuse people. I already in a previous version of this series
named linear_page_delta() as linear_page_offset() and then changed it to avoid
confusion.
And I'm not sure it's really all that useful. Perhaps retaining vma_offset()
would be though.
2.
This is a much more useful form I noticed, effectively drivers doing the inverse
of a linear_page_index() to get the address:
static inline unsigned long linear_page_address(const struct vm_area_struct
*vma,
const pgoff_t pgoff)
{
const pgoff_t page_delta = pgoff - vma_start_pgoff(vma);
const unsigned long offset = page_delta << PAGE_SHIFT;
return vma->vm_start + offset;
}
This is one that I think makes more sense.
But in general, I'd rather hold off from yet more churn here.
I'm making these changes to establish a basis for virtual page offsets
introduced in [0], rather than just cleaning up in general.
These changes are really to make it such that we more consistently use these
forms, so when I introduce the virt pgoff versions, it fits canonically into
that.
And also beacuse I may as well improve kernel code as I go :)
But I think adding yet more doesn't really serve the same purpose.
But it's food for a follow up perhaps?
>
> >
> > /**
> > @@ -1482,7 +1484,7 @@ static int unapply_uprobe(struct uprobe *uprobe,
> > struct mm_struct *mm)
> > file_inode(vma->vm_file) != uprobe->inode)
> > continue;
> >
> > - offset = (loff_t)vma->vm_pgoff << PAGE_SHIFT;
> > + offset = (loff_t)vma_start_pgoff(vma) << PAGE_SHIFT;
> > if (uprobe->offset < offset ||
> > uprobe->offset >= offset + vma->vm_end - vma->vm_start)
> > continue;
> > @@ -2453,7 +2455,8 @@ static struct uprobe
> > *find_active_uprobe_speculative(unsigned long bp_vaddr)
> > if (!vm_file)
> > return NULL;
> >
> > - offset = (loff_t)(vma->vm_pgoff << PAGE_SHIFT) + (bp_vaddr -
> > vma->vm_start);
> > + offset = (loff_t)(vma_start_pgoff(vma) << PAGE_SHIFT) +
> > + (bp_vaddr - vma->vm_start);
>
> This is more extremely contrived logic that could be better expressed as
>
> loff_t vma_linear_off(vma, bp_vaddr);
See above.
>
> > uprobe = find_uprobe_rcu(vm_file->f_inode, offset);
> > if (!uprobe)
> > return NULL;
> > diff --git a/kernel/kcov.c b/kernel/kcov.c
> > index 1df373fb562b..b19b473c366a 100644
> > --- a/kernel/kcov.c
> > +++ b/kernel/kcov.c
> > @@ -512,7 +512,7 @@ static int kcov_mmap(struct file *filep, struct
> > vm_area_struct *vma)
> >
> > spin_lock_irqsave(&kcov->lock, flags);
> > size = kcov->size * sizeof(unsigned long);
> > - if (kcov->area == NULL || vma->vm_pgoff != 0 ||
> > + if (kcov->area == NULL || vma_start_pgoff(vma) ||
>
> as a nit, perhaps vma_start_pgoff(vma) > 0
> would be a little more idiomatic.
I felt the if (<val>) form was more idiomatic?
>
> > vma->vm_end - vma->vm_start != size) {
> > res = -EINVAL;
> > goto exit;
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 56a328e94395..dfa493d54ef9 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -7613,7 +7613,8 @@ static int __rb_inc_dec_mapped(struct
> > ring_buffer_per_cpu *cpu_buffer,
> > static int __rb_map_vma(struct ring_buffer_per_cpu *cpu_buffer,
> > struct vm_area_struct *vma)
> > {
> > - unsigned long nr_subbufs, nr_pages, nr_vma_pages, pgoff = vma->vm_pgoff;
> > + unsigned long nr_subbufs, nr_pages, nr_vma_pages;
> > + pgoff_t pgoff = vma_start_pgoff(vma);
> > unsigned int subbuf_pages, subbuf_order;
> > struct page **pages __free(kfree) = NULL;
> > int p = 0, s = 0;
>
> Anyway, in general:
>
> Acked-by: Pedro Falcato <[email protected]>
Thanks!
>
> --
> Pedro
[0]:https://lore.kernel.org/linux-mm/[email protected]/
