[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250115085959.GD8385@noisy.programming.kicks-ass.net>
Date: Wed, 15 Jan 2025 09:59:59 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: akpm@...ux-foundation.org, willy@...radead.org, liam.howlett@...cle.com,
lorenzo.stoakes@...cle.com, mhocko@...e.com, vbabka@...e.cz,
hannes@...xchg.org, mjguzik@...il.com, oliver.sang@...el.com,
mgorman@...hsingularity.net, david@...hat.com, peterx@...hat.com,
oleg@...hat.com, dave@...olabs.net, paulmck@...nel.org,
brauner@...nel.org, dhowells@...hat.com, hdanton@...a.com,
hughd@...gle.com, lokeshgidra@...gle.com, minchan@...gle.com,
jannh@...gle.com, shakeel.butt@...ux.dev, souravpanda@...gle.com,
pasha.tatashin@...een.com, klarasmodin@...il.com,
richard.weiyang@...il.com, corbet@....net,
linux-doc@...r.kernel.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH v8 00/16] move per-vma lock into vm_area_struct
On Fri, Jan 10, 2025 at 06:01:05PM +0100, Peter Zijlstra wrote:
> On Thu, Jan 09, 2025 at 07:48:32AM -0800, Suren Baghdasaryan wrote:
> > On Thu, Jan 9, 2025 at 3:51 AM Peter Zijlstra <peterz@...radead.org> wrote:
> > >
> > > On Wed, Jan 08, 2025 at 06:30:09PM -0800, Suren Baghdasaryan wrote:
> > > > Back when per-vma locks were introduces, vm_lock was moved out of
> > > > vm_area_struct in [1] because of the performance regression caused by
> > > > false cacheline sharing. Recent investigation [2] revealed that the
> > > > regressions is limited to a rather old Broadwell microarchitecture and
> > > > even there it can be mitigated by disabling adjacent cacheline
> > > > prefetching, see [3].
> > > > Splitting single logical structure into multiple ones leads to more
> > > > complicated management, extra pointer dereferences and overall less
> > > > maintainable code. When that split-away part is a lock, it complicates
> > > > things even further. With no performance benefits, there are no reasons
> > > > for this split. Merging the vm_lock back into vm_area_struct also allows
> > > > vm_area_struct to use SLAB_TYPESAFE_BY_RCU later in this patchset.
> > > > This patchset:
> > > > 1. moves vm_lock back into vm_area_struct, aligning it at the cacheline
> > > > boundary and changing the cache to be cacheline-aligned to minimize
> > > > cacheline sharing;
> > > > 2. changes vm_area_struct initialization to mark new vma as detached until
> > > > it is inserted into vma tree;
> > > > 3. replaces vm_lock and vma->detached flag with a reference counter;
> > > > 4. changes vm_area_struct cache to SLAB_TYPESAFE_BY_RCU to allow for their
> > > > reuse and to minimize call_rcu() calls.
> > >
> > > Does not clean up that reattach nonsense :-(
> >
> > Oh, no. I think it does. That's why in [1] I introduce
> > vma_iter_store_attached() to be used on already attached vmas and to
> > avoid marking them attached again. Also I added assertions in
> > vma_mark_attached()/vma_mark_detached() to avoid re-attaching or
> > re-detaching. Unless I misunderstood your comment?
>
> Hmm, I'll go read the thing again, maybe I missed it.
You're right. I was looking for the approach that changed the need to
reattach, by moving the point of no return.
This should do for now.
Let me see if I can find time today to finally do a proper reading.
Thanks!
Powered by blists - more mailing lists