Re: [PATCH v3 08/13] mm: add ability to take further action in vm_area_desc

Lorenzo Stoakes
2025-09-16T19:32:44Z

On Tue, Sep 16, 2025 at 03:08:54PM -0300, Jason Gunthorpe wrote:
> On Tue, Sep 16, 2025 at 06:57:56PM +0100, Lorenzo Stoakes wrote:
> > > > + /*
> > > > + * If an error occurs, unmap the VMA altogether and return an error. We
> > > > + * only clear the newly allocated VMA, since this function is only
> > > > + * invoked if we do NOT merge, so we only clean up the VMA we created.
> > > > + */
> > > > + if (err) {
> > > > + const size_t len = vma_pages(vma) << PAGE_SHIFT;
> > > > +
> > > > + do_munmap(current->mm, vma->vm_start, len, NULL);
> > > > +
> > > > + if (action->error_hook) {
> > > > + /* We may want to filter the error. */
> > > > + err = action->error_hook(err);
> > > > +
> > > > + /* The caller should not clear the error. */
> > > > + VM_WARN_ON_ONCE(!err);
> > > > + }
> > > > + return err;
> > > > + }
> > > Also seems like this cleanup wants to be in a function that is not
> > > protected by #ifdef nommu since the code is identical on both branches.
> >
> > Not sure which cleanup you mean, this is new code :)
>
> I mean the code I quoted right abouve that cleans up the VMA on
> error.. It is always the same finishing sequence, there is no nommu
> dependency in it.

Ah right yeah.

I wonder if it's useful if err != 0 for nommu anyway.

>
> Just put it all in some "finish mmap complete" function and call it in
> both mmu and nommu versions.

Ack!

>
> Jason
>

Cheers, Lorenzo