Re: [PATCH v1] kernel/fork: only call untrack_pfn_clear() on VMAs duplicated for fork()
From: Lorenzo Stoakes
Date: Thu Apr 24 2025 - 08:50:48 EST
On Thu, Apr 24, 2025 at 02:33:54PM +0200, David Hildenbrand wrote:
> > >
> > > ... and I think we still have space in vm_area_struct without increasing it
> > > beyond 192 bytes.
> >
> > Hm, so you're thinking of a general field in the VMA? I thought this would
> > belong to the PAT object somehow?
>
> It's glued to a VMA. The only alternative to using a VMA field would be looking it
> up for a VMA, storing it in an xarray etc ... ends up complicating stuff when there
> is no need to right now.
Yeah no, that'd be crazy.
I guess we are stuck with this so need to think of a sensible way forward.
>
> >
> > Though getting rid of VM_PAT would be fantastic...
> >
> > I wonder if a _general_ VMA ref count would be a bit much just for this
> > case.
>
> I don't think it would be helpful for this case. It's much more similar to the anon
> VMA name (that also has its own kref)
Ahh I see, adding a field like this, that makes a lot more sense, obviously
could put behind appropriate CONFIG_ flag.
Given cacheline alignment probably have a bit of wiggle room there to avoid
increasing VMA size also.
>
> >
> > But maybe I misunderstand your approach :) Happy to obviously look and if
> > not like some crazy thing just for PAT (you can understand why I would not
> > like this) will be supportive :>)
>
> This is something quick (well, longer than I wish it would take) that seems to
> work. There are smaller pat-internal cleanups to be had on top of this, and
> the new functions shall be documented.
>
>
> Observe how:
> * We remove VM_PAT and that weird VM flags manipulation + "locked" flag
> * We remove any traces of the nasty tracking handling from mremap+fork code
> * Just like anon_vma_name, it hooks into vm_area_dup()/vm_area_free().
> * We remove the page table lookup via get_pat_info()->... completely
> * We remove the VMA parameter from PAT code completely
> * We reduce the track/untrack/sanitize interface to 3 functions
Yeah this is all lovely!
OK this should hopefully be workable then!