Re: [PATCH v2 0/3] use vma locks for proc/pid/{smaps|numa_maps} reads

Re: [PATCH v2 0/3] use vma locks for proc/pid/{smaps|numa_maps} reads

[David Hildenbrand (Arm)]

On 4/26/26 08:27, Suren Baghdasaryan wrote:
> Use per-vma locks when reading /proc/pid/smaps and /proc/pid/numa_maps
> similar to /proc/pid/maps to reduce contention on central mmap_lock. One
> major difference between maps and smaps/numa_maps reading is that the
> latter executes page table walk which can't be done under RCU due to a
> possibility of sleeping. Therefore we drop RCU read lock before this walk
> while keeping the VMA locked. After the walk we retake RCU read lock,
> reset VMA iterator and proceed with the next VMA.

With many small VMAs, is that overhead noticable?

-- 
Cheers,

David

Re: [PATCH v2 0/3] use vma locks for proc/pid/{smaps|numa_maps} reads

[Suren Baghdasaryan]

On Sun, Apr 26, 2026 at 12:59 PM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Sat, 25 Apr 2026 23:27:15 -0700 Suren Baghdasaryan <surenb@google.com> wrote:
>
> > Use per-vma locks when reading /proc/pid/smaps and /proc/pid/numa_maps
> > similar to /proc/pid/maps to reduce contention on central mmap_lock.
>
> Was the benefit measured/measurable?
>
> Sashiko doesn't like your strchr():
>         https://sashiko.dev/#/patchset/20260426062718.1238437-1-surenb@google.com

Sorry, missed your comment earlier.

I think strchr() is fine here and Sashiko did notice that read_page()
always makes sure the buffer ends with a newline character, IOW
*end_pos == '\n'. So, once we found this last newline and moved past
it, we terminate the loop. I guess Sashiko is being overprotective...

Re: [PATCH v2 0/3] use vma locks for proc/pid/{smaps|numa_maps} reads

[Suren Baghdasaryan]

On Tue, May 12, 2026 at 2:28 AM David Hildenbrand (Arm)
<david@kernel.org> wrote:
>
> On 4/26/26 08:27, Suren Baghdasaryan wrote:
> > Use per-vma locks when reading /proc/pid/smaps and /proc/pid/numa_maps
> > similar to /proc/pid/maps to reduce contention on central mmap_lock. One
> > major difference between maps and smaps/numa_maps reading is that the
> > latter executes page table walk which can't be done under RCU due to a
> > possibility of sleeping. Therefore we drop RCU read lock before this walk
> > while keeping the VMA locked. After the walk we retake RCU read lock,
> > reset VMA iterator and proceed with the next VMA.
>
> With many small VMAs, is that overhead noticable?

It might be but the point of this patchset (and the previous one that
made a similar change for /proc/pid/maps) is to reduce mmap_lock
contention, not to speed up the read operation, which is not a
performance critical part. The original problem that Paul McKenney
described and which kicked these series of changes is that a
low-priority monitoring process reading /proc/pid/{maps|smaps|...} can
block a high-priority updates by holding the mmap_lock. You can see
details about this problem and the numbers Paul obtained with the
previous change in here:
https://lore.kernel.org/all/20250719182854.3166724-1-surenb@google.com/

>
> --
> Cheers,
>
> David

Re: [PATCH v2 0/3] use vma locks for proc/pid/{smaps|numa_maps} reads

[David Hildenbrand (Arm)]

On 5/13/26 05:58, Suren Baghdasaryan wrote:
> On Tue, May 12, 2026 at 2:28 AM David Hildenbrand (Arm)
> <david@kernel.org> wrote:
>>
>> On 4/26/26 08:27, Suren Baghdasaryan wrote:
>>> Use per-vma locks when reading /proc/pid/smaps and /proc/pid/numa_maps
>>> similar to /proc/pid/maps to reduce contention on central mmap_lock. One
>>> major difference between maps and smaps/numa_maps reading is that the
>>> latter executes page table walk which can't be done under RCU due to a
>>> possibility of sleeping. Therefore we drop RCU read lock before this walk
>>> while keeping the VMA locked. After the walk we retake RCU read lock,
>>> reset VMA iterator and proceed with the next VMA.
>>
>> With many small VMAs, is that overhead noticable?
> 
> It might be but the point of this patchset (and the previous one that
> made a similar change for /proc/pid/maps) is to reduce mmap_lock
> contention, not to speed up the read operation, which is not a
> performance critical part.

Well, this interface has been around .. forever, so if there is a noticeable
change in performance it should be called out.

> The original problem that Paul McKenney
> described and which kicked these series of changes is that a
> low-priority monitoring process reading /proc/pid/{maps|smaps|...} can
> block a high-priority updates by holding the mmap_lock. You can see
> details about this problem and the numbers Paul obtained with the
> previous change in here:
> https://lore.kernel.org/all/20250719182854.3166724-1-surenb@google.com/

Yes, I know. A problem that has been around ... forever as well :)

-- 
Cheers,

David