RE: [PATCH 0/15] mm: introduce ANON_VMA_LAZY for deferred anon_vma creation

[wangtao <tao.wangtao@honor.com>]

> >
> > Hi!
> >
> > When I saw the diffsat I was concerned. Going through the patches made
> me ...
> > more concerned :)
> >
> > This is a lot of complexity. On top of something that is already so
> > complicated that I fail to grasp most details without regularly taking
> > a look at the nice figures Lorenzo created recently.
> >
> > For example, I read above "since child VMAs do not need to allocate
> anon_vma"
> > and wondered how that could be part of something that is just done
> > lazily. Then I had to learn in the patches that there is some
> > additional "Child VMAs are created as ANON_VMA_TREE_PARENT and do
> not
> > allocate anon_vma" -- excuse me, what? :)
> 
> Yes, that part is quite complicated here. There are two cases here:
> 1. A forks B, and B inherits a VMA from A. In this case, B's VMA gets
> ANON_VMA_TREE_PARENT.
> 
> 2. A forks B, and B later creates a new VMA via mmap().
> If a page fault occurs in this new VMA, it gets ANON_VMA_TREE_VMA.
> 
> In both cases, we need to upgrade B to a regular anon_vma when B becomes
> a parent and performs a fork().
> 
Thank you for helping explain.

> This may be a bit off-topic, but I'm also considering whether there is a chance
> to work with Suren to support case 2 via a GKI hook in the Android kernel
> before Lorenzo's work is ready.
> 
> Even then, the optimization would apply only to the case where B never
> forks, allowing us to skip the anon_vma "upgrade" entirely. That assumption
> holds for most applications, although there are a few cases where it does not.
> 
> I'm actually hoping Android could eventually disable forking for UI
> applications altogether. From what I've heard, some applications use fork()
> primarily to evade LMKD (the Android low-memory killer daemon). For
> example, a child process may monitor the main process, and if the main
> process is killed, detect that event and request a relaunch. This is one way
> some applications attempt to keep themselves alive indefinitely.
> 
> But even if we limit the optimization to the subset of case 2 where B never
> forks, we still need to handle mremap(), VMA merges, VMA splits, and
> similar cases.
> That starts to become quite a headache.
> 
> So please just ignore my rambling if it turns out to be nonsense :-)
> 
> >
> > Reading about VMA refcounts made me shiver. Reading "Holding only
> > folio_lock(folio) cannot guarantee that the split operation completes
> > atomically." confused me. Learning that we have to invent interesting
> > ways to make page migration mutually exclusive to free_pgtables()
> > concerned me. Figuring out that there are arch-specific config options
> > and runtime toggles is a clear warning sign.
> >
> > Seeing test_folio_unmapped() was funny, though (why?! :)).
> >
> > I think this patch set has a noble goal of reducing anon_vma overhead
> > when anon pages are not shared during fork. However, using anon_vma
> > for them actually makes the overall implementation (e.g., rmap walks,
> > locking) more consistent and simpler.
> >
> > Even if we could be convinced that most of this here is correct, how
> > should we reasonably maintain this increasing level of complexity here?
> >
> > I won't echo what has already been said in this thread (and I didn't
> > manage to read all, unfortunately), but for such big and invasive work
> > it's often best to get in touch with the community earlier. Otherwise,
> > you might end up wasting your time.
> >
> > Ok, arguably, someone who writes that code learns a lot on the way.
> > And if this code really was written by one developer only, I tip my
> > hat! I'd be curious if that code already ran somewhere on some Android
> kernel out there?
> 
> I heard from Zicheng that they have been running this for months and it
> seems reasonably stable. Please correct me if I'm wrong, Zicheng :-). This
> really should have been discussed with the community earlier.
> 
I initially developed and debugged this based on the Android GKI branch
and did some preliminary testing on an Android phone.

Since GKI generally only accepts features merged from the upstream
community, and this memory saving could also benefit the community, I
ported the patch to the Linux master branch.

Because my English is not very good and I rarely participate in the
community, I am not familiar with the community workflow. I did not send
an email for discussion in advance with an RFC tag. I apologize again.

> >
> > But adding more complexity on top of something that's already
> > extremely complicated to save some memory looks like the wrong
> direction, really.
> >
> > I was excited when Lorenzo started working on a completely new
> > approach that would focus on improving the common cases while trying
> > to reduce the overall complexity. Because I think most of us really
> > dislike anon_vma. It's still work in progress, and I am sure there are some
> rough edges.
> >
> > But fundamentally, I think we want to find a new design that is just
> > naturally simpler.
> >
> 
> +1
> 
> > Lorenzo has been hard at work exploring various design options (and
> > I'm afraid he might be one of the 3 people on this planet that
> > understand anon_vma in full detail), so I suggest we wait for a
> > redesign proposal from him and see if that is doable?
> >
> 
> Thanks
> Barry