Message-ID: <29510ad8-a240-4ab8-8af8-75e3f377da78@lucifer.local>
Date: Mon, 15 Sep 2025 10:56:57 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Chris Mason <clm@...a.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>, Matthew Wilcox <willy@...radead.org>,
        Guo Ren <guoren@...nel.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Sven Schnelle <svens@...ux.ibm.com>,
        "David S . Miller" <davem@...emloft.net>,
        Andreas Larsson <andreas@...sler.com>, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        Dave Jiang <dave.jiang@...el.com>, Nicolas Pitre <nico@...xnic.net>,
        Muchun Song <muchun.song@...ux.dev>,
        Oscar Salvador <osalvador@...e.de>,
        David Hildenbrand <david@...hat.com>,
        Konstantin Komarov <almaz.alexandrovich@...agon-software.com>,
        Baoquan He <bhe@...hat.com>, Vivek Goyal <vgoyal@...hat.com>,
        Dave Young <dyoung@...hat.com>, Tony Luck <tony.luck@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Dave Martin <Dave.Martin@....com>, James Morse <james.morse@....com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
        Hugh Dickins <hughd@...gle.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        Uladzislau Rezki <urezki@...il.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Andrey Konovalov <andreyknvl@...il.com>, Jann Horn <jannh@...gle.com>,
        Pedro Falcato <pfalcato@...e.de>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-csky@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-s390@...r.kernel.org, sparclinux@...r.kernel.org,
        nvdimm@...ts.linux.dev, linux-cxl@...r.kernel.org, linux-mm@...ck.org,
        ntfs3@...ts.linux.dev, kexec@...ts.infradead.org,
        kasan-dev@...glegroups.com, Jason Gunthorpe <jgg@...dia.com>
Subject: Re: [PATCH v2 08/16] mm: add ability to take further action in
 vm_area_desc

On Sat, Sep 13, 2025 at 06:54:06PM -0400, Chris Mason wrote:
> Hi Lorzeno,
>
> On 9/10/25 4:22 PM, Lorenzo Stoakes wrote:
> > Some drivers/filesystems need to perform additional tasks after the VMA is
> > set up. This is typically in the form of pre-population.
> >
> > The forms of pre-population most likely to be performed are a PFN remap or
> > insertion of a mixed map, so we provide this functionality, ensuring that
> > we perform the appropriate actions at the appropriate time - that is
> > setting flags at the point of .mmap_prepare, and performing the actual
> > remap at the point at which the VMA is fully established.
> >
> > This prevents the driver from doing anything too crazy with a VMA at any
> > stage, and we retain complete control over how the mm functionality is
> > applied.
> >
> > Unfortunately callers still do often require some kind of custom action, so
> > we add an optional success/error _hook to allow the caller to do something
> > after the action has succeeded or failed.
> >
> > This is done at the point when the VMA has already been established, so the
> > harm that can be done is limited.
> >
> > The error hook can be used to filter errors if necessary.
> >
> > We implement actions as abstracted from the vm_area_desc, so we provide the
> > ability for custom hooks to invoke actions distinct from the vma
> > descriptor.
> >
> > If any error arises on these final actions, we simply unmap the VMA
> > altogether.
> >
> > Also update the stacked filesystem compatibility layer to utilise the
> > action behaviour, and update the VMA tests accordingly.
> >
> > For drivers which perform truly custom logic, we provide a custom action
> > hook which is invoked at the point of action execution.
> >
> > This can then, in turn, update the desc object and perform other actions,
> > such as partially remapping ranges for instance. We export
> > vma_desc_action_prepare() and vma_desc_action_complete() for drivers to do
> > this.
> >
> > This is performed at a stage where the VMA is already established,
> > immediately prior to mapping completion, so it is considerably less
> > problematic than a general mmap hook.
> >
> > Note that at the point of the action being taken, the VMA is visible via
> > the rmap, only the VMA write lock is held, so if anything needs to access
> > the VMA, it is able to.
> >
> > Essentially the action is taken as if it were performed after the mapping,
> > but is kept atomic with VMA state.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> > ---
> >  include/linux/mm.h               |  30 ++++++
> >  include/linux/mm_types.h         |  61 ++++++++++++
> >  mm/util.c                        | 150 +++++++++++++++++++++++++++-
> >  mm/vma.c                         |  70 ++++++++-----
> >  tools/testing/vma/vma_internal.h | 164 ++++++++++++++++++++++++++++++-
> >  5 files changed, 447 insertions(+), 28 deletions(-)
> >
>
> [ ... ]
>
> > +/**
> > + * mmap_action_complete - Execute VMA descriptor action.
> > + * @action: The action to perform.
> > + * @vma: The VMA to perform the action upon.
> > + *
> > + * Similar to mmap_action_prepare(), other than internal mm usage this is
> > + * intended for mmap_prepare users who implement a custom hook - with this
> > + * function being called from the custom hook itself.
> > + *
> > + * Return: 0 on success, or error, at which point the VMA will be unmapped.
> > + */
> > +int mmap_action_complete(struct mmap_action *action,
> > +			     struct vm_area_struct *vma)
> > +{
> > +	int err = 0;
> > +
> > +	switch (action->type) {
> > +	case MMAP_NOTHING:
> > +		break;
> > +	case MMAP_REMAP_PFN:
> > +		VM_WARN_ON_ONCE((vma->vm_flags & VM_REMAP_FLAGS) !=
> > +				VM_REMAP_FLAGS);
> > +
> > +		err = remap_pfn_range_complete(vma, action->remap.addr,
> > +				action->remap.pfn, action->remap.size,
> > +				action->remap.pgprot);
> > +
> > +		break;
> > +	case MMAP_INSERT_MIXED:
> > +	{
> > +		unsigned long pgnum = 0;
> > +		unsigned long pfn = action->mixedmap.pfn;
> > +		unsigned long addr = action->mixedmap.addr;
> > +		unsigned long vaddr = vma->vm_start;
> > +
> > +		VM_WARN_ON_ONCE(!(vma->vm_flags & VM_MIXEDMAP));
> > +
> > +		for (; pgnum < action->mixedmap.num_pages;
> > +		     pgnum++, pfn++, addr += PAGE_SIZE, vaddr += PAGE_SIZE) {
> > +			vm_fault_t vmf;
> > +
> > +			vmf = vmf_insert_mixed(vma, vaddr, addr);
>                                                           ^^^^^
> Should this be pfn instead of addr?

Yeah, sigh, this is a direct product of cramfs seemingly having a bug where it
was passing PA's and not PFNs.

I thought I had fixed this but clearly I missed this here.

Let me send a fix-patch!

>
> -chris

Cheers, Lorenzo

Powered by blists - more mailing lists