From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D60E04189DE; Thu, 2 Jul 2026 11:31:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782991883; cv=none; b=PBj0u+mKWlBWLQhCMZkoJAUx083QEyd4zsFlEGxPeTis1ShibrUoawyZ4VYVB4tWTd6n+KEhzPFlN8ydabRPq4LZQwZ/t3nsY0jp5+HGmdnKZ48g6smCZXxBDigp//TJnd2swDxEPDo+krsQ19QCY3yLcqakZCYc76I7ImfCXiQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782991883; c=relaxed/simple; bh=oEqsGD08xxe0Cu5FosBKzGaCg+/FdysJ7yQ46TYIhfw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pPVaQFawKGbR9mu5cYi0IAu0kTZ3KuKj9CIuSBmC/tJxZakwi7jLBbPJcs3TXIDMepKj16Ofv2AhG/ATdr1EQhw3IAs8W/uRVgCbUvLnhuAnQMCGlK7vg9auGAeyu3yAqprLB+vKAvifiyv6HSw4OmkpqAEdgHziZxgkfjURGcg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KF5raoni; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KF5raoni" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 57DC61F000E9; Thu, 2 Jul 2026 11:31:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782991881; bh=dwzYYCSTerfam2cyFhh651QlnQHZp+3k2k49hku8SSY=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=KF5raoniPxVs+VeUV8vJZ742SCHPFlqTXauJ93LdE8Wk4u2oT/RTikIryvzEjoFTM c+iETtRrKPNLW25+aVFyh+UInETHjtdNJA7RaGGgPjuPfA7mIbbsXSwJcEOhXFB27q tRapybkQ1EbsGMOGwaRTTqphy/6in3beInqSfoQj1KGxFyzVLjB87vyd4UI8rLuZtm B15PeA2wTphKmPXgyQ22V97dpT12OorTrt8nJcQxc2aZ1U7R0lFfri1r2w7gw73NE4 V8r067lErBLwwsU5agFpHddFROV4sLr7eRYllW0m1kbIinebUf8sVbUlC34ZWLlWDU kbjN+G14+uopw== Date: Thu, 2 Jul 2026 12:30:59 +0100 From: Lorenzo Stoakes To: Pedro Falcato Cc: Andrew Morton , Russell King , Dinh Nguyen , Simon Schuster , "James E . J . Bottomley" , Helge Deller , Jarkko Sakkinen , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, Ian Abbott , H Hartley Sweeten , Lucas Stach , David Airlie , Simona Vetter , Patrik Jakobsson , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Rob Clark , Dmitry Baryshkov , Tomi Valkeinen , Thierry Reding , Mikko Perttunen , Jonathan Hunter , Christian Koenig , Huang Rui , Ankit Agrawal , Alex Williamson , Alexander Viro , Christian Brauner , Dan Williams , Muchun Song , Oscar Salvador , David Hildenbrand , Suren Baghdasaryan , "Liam R . Howlett" , Matthew Wilcox , Marek Szyprowski , Peter Zijlstra , Arnaldo Carvalho de Melo , Namhyung Kim , Masami Hiramatsu , Oleg Nesterov , Steven Rostedt , SeongJae Park , Miaohe Lin , Hugh Dickins , Mike Rapoport , Kees Cook , Paolo Bonzini , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-parisc@vger.kernel.org, linux-sgx@vger.kernel.org, etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, linux-tegra@vger.kernel.org, kvm@vger.kernel.org, linux-fsdevel@vger.kernel.org, nvdimm@lists.linux.dev, linux-mm@kvack.org, iommu@lists.linux.dev, linux-perf-users@vger.kernel.org, linux-trace-kernel@vger.kernel.org, kasan-dev@googlegroups.com, damon@lists.linux.dev, Rik van Riel , Harry Yoo , Jann Horn Subject: Re: [PATCH 17/30] mm: prefer vma_[start,end]_pgoff() to vma->vm_pgoff in kernel/ Message-ID: References: Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 > > --- > > 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 () 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 Thanks! > > -- > Pedro [0]:https://lore.kernel.org/linux-mm/cover.1782745153.git.ljs@kernel.org/