Re: [PATCH mm/bpf v2] mm: bpf: add find_vma_no_check() without lockdep_assert on mm->mmap_lock

[Liam Howlett <liam.howlett@oracle.com>]

* Alexei Starovoitov <alexei.starovoitov@gmail.com> [210908 14:03]:
> On Wed, Sep 08, 2021 at 10:52:59AM -0700, Andrew Morton wrote:
> > On Wed, 8 Sep 2021 10:21:18 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > 
> > > > Again I am ignorant on the details so if you can clarify the following
> > > > it may help me and others to better understand the problem:
> > > > 
> > > > 1. Peter's patch appears to just take the same "fallback" path
> > > >    that would be taken if the trylock failed.
> > > >    Is this really a breakage or just loss of performance ?
> > > >    I would expect the latter, since it is called "fallback".
> > > 
> > > As Yonghong explained it's a user space breakage.
> > > User space tooling expects build_id to be available 99.999% of the time
> > > and that's what users observed in practice.
> > > They've built a bunch of tools on top of this feature.
> > > The data from these tools goes into various datacenter tables
> > > and humans analyze it later.
> > > So Peter's proposal is not acceptable. We don't want to get yelled at.
> > > 
> > 
> > I'm not understanding.  Peter said "this patch merely removes a
> > performance tweak" and you and Yonghong said "it breaks userspace". 
> > These assertions are contradictory!
> 
> Peter said:
> "The only sane approach is making the vma tree lockless, but so far the
>  bpf people have resisted doing the right thing because they've been
>  allowed to get away with these atrocities.
> "
> which is partially true.
> bpf folks didn't resist it. There is work ongoing to make it lockless.
> It just takes an long time. I don't see how bpf folks can speed it up
> any further.

What work are you doing on a lockless vma tree?  I've been working on
the maple tree and would like to hear what you've come up with.

> 
> > Please describe the expected userspace-visible change from Peter's
> > patch in full detail?
> 
> User space expects build_id to be available. Peter patch simply removes
> that feature.
> 
> > And yes, it is far preferable that we resolve this by changing BPF to
> > be a better interface citizen, please.  Let's put those thinking caps on?
> 
> Just silence a lockdep as Yonghong proposed or some other way,
> since it's only a lockdep issue. There is no actual breakage.
> The feature was working and still works as intended.