Re: [PATCH 06/16] mm: introduce the f_op->mmap_complete, mmap_abort hooks

Suren Baghdasaryan
2025-09-09T16:43:40Z

On Tue, Sep 9, 2025 at 2:37 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Tue, Sep 09, 2025 at 11:26:21AM +0200, David Hildenbrand wrote:
> > > >
> > > > In particular, the mmap_complete() looks like another candidate for letting
> > > > a driver just go crazy on the vma? :)
> > >
> > > Well there's only so much we can do. In an ideal world we'd treat VMAs as
> > > entirely internal data structures and pass some sort of opaque thing around, but
> > > we have to keep things real here :)
> >
> > Right, we'd pass something around that cannot be easily abused (like
> > modifying random vma flags in mmap_complete).
> >
> > So I was wondering if most operations that driver would perform during the
> > mmap_complete() could be be abstracted, and only those then be called with
> > whatever opaque thing we return here.
>
> Well there's 2 issues at play:
>
> 1. I might end up having to rewrite _large parts_ of kernel functionality all of
> which relies on there being a vma parameter (or might find that to be
> intractable).
>
> 2. There's always the 'odd ones out' :) so there'll be some drivers that
> absolutely do need to have access to this.
>
> But as I was writing this I thought of an idea - why don't we have something
> opaque like this, perhaps with accessor functions, but then _give the ability to
> get the VMA if you REALLY have to_.
>
> That way we can handle both problems without too much trouble.
>
> Also Jason suggested generic functions that can just be assigned to
> .mmap_complete for instance, which would obviously eliminate the crazy
> factor a lot too.
>
> I'm going to refactor to try to put ONLY prepopulate logic in
> .mmap_complete where possible which fits with all of this.

Thinking along these lines, do you have a case when mmap_abort() needs
vm_private_data? I was thinking if VMA mapping failed, why would you
need vm_private_data to unwind prep work? You already have the context
pointer for that, no?

>
> >
> > But I have no feeling about what crazy things a driver might do. Just
> > calling remap_pfn_range() would be easy, for example, and we could abstract
> > that.
>
> Yeah, I've obviously already added some wrappers for these.
>
> BTW I really really hate that STUPID ->vm_pgoff hack, if not for that, life
> would be much simpler.
>
> But instead now we need to specify PFN in the damn remap prepare wrapper in
> case of CoW. God.
>
> >
> > --
> > Cheers
> >
> > David / dhildenb
> >
>
> Cheers, Lorenzo