Re: [PATCH 2/2] fs/proc/task_mmu: read proc/pid/smaps_rollup under per-vma lock

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/10/26 16:56, Suren Baghdasaryan wrote:
> On Wed, Jun 10, 2026 at 12:05 AM Lorenzo Stoakes <ljs@kernel.org> wrote:
>>
>> On Tue, Jun 09, 2026 at 10:11:03AM -0700, Suren Baghdasaryan wrote:
>>>
>>> Ack.
>>>
>>>
>>> Ack.
>>>
>>>
>>> Sadly, for now we do (see my later comment).
>>>
>>>
>>> It looked nicer I think :)
>>
>> Yeah tbh I think you're right
>>
>>>
>>>
>>> Ack.
>>>
>>>
>>> We need it to avoid resetting vma_start, which is being reported even
>>> when no normal VMAs are present.
>>
>> Well previously smap_gather_stats() would just be a no-op for a gate VMA, as it
>> has:
>>
>>         if (vma == get_gate_vma(priv->lock_ctx.mm))
>>                 return;
>>
>> Not sure if you'd have VMAs past that (is that even possible?) but in that case
>> we would have set vma_start previously.
> 
> I should explain this case in more detail. proc_get_vma() returns a
> gate VMA once it finds no more VMAs. So, in this case if VMA tree is
> empty, the first call to proc_get_vma() will return gate VMA. Now, the
> original code was using vma_next() and would return NULL when the VMA
> tree is empty and in that case it would bail out early here:
> https://elixir.bootlin.com/linux/v7.1-rc7/source/fs/proc/task_mmu.c#L1423.
> I need to use proc_get_vma() instead of vma_next() because it works
> under RCU. So, the new code matches the previous behavior, but the
> condition is different. I'll see if I can resolve this in a nicer way
> but that's the gist of it.

I guess some well-named helper that encapsulates the check would already help.
But maybe the whole logic can be overhauled.

-- 
Cheers,

David