Re: [PATCH v2 4/5] proc/task_mmu: read proc/pid/smaps_rollup under per-vma lock
From: Suren Baghdasaryan
Date: Thu Sep 10 2026 - 17:25:29 EST
On Thu, Sep 10, 2026 at 12:44 AM David Hildenbrand (Arm)
<david@xxxxxxxxxx> wrote:
>
> On 9/9/26 19:58, Suren Baghdasaryan wrote:
> > On Wed, Sep 9, 2026 at 10:23 AM David Hildenbrand (Arm)
> > <david@xxxxxxxxxx> wrote:
> >>
> >>
> >>> - vma_start = vma->vm_start;
> >>> - do {
> >>> - smap_gather_stats(priv, vma, &mss, vma->vm_start);
> >>> - last_vma_end = vma->vm_end;
> >>> + if (!IS_ERR(vma) && vma != get_gate_vma(lock_ctx->mm))
> >>> + vma_start = vma->vm_start;
> >>> +
> >>> + while (vma) {
> >>> + if (IS_ERR(vma)) {
> >>> + ret = PTR_ERR(vma);
> >>> + goto out_unlock;
> >>> + }
> >>> +
> >>
> >> Can we add a comment whey we break (and not e.g., continue) whenw e hit the gate
> >> VMA?
> >>
> >> (I seriously don't kmow ... should I know? :) )
> >
> > The way m_next() is implemented, the gate VMA always placed at the end
> > of the address space, so the next VMA will be NULL and we can break
> > once we see the gate. But now that I'm looking closer into this code,
> > reading smaps_rollup file does not invoke m_next(), so we should never
> > encounter a gate VMA (it's not in the maple tree, so for_each_vma()
> > should never return it). I think I can remove the special handling for
> > that case.
> >
> > Thanks for the question, David! It made me realize we can simplify this further.
>
> good! :)
Sorry, my conclusion was wrong. The modified loop uses proc_get_vma()
to walk the VMA tree, not for_each_vma(). proc_get_vma() returns gate
VMA at the end of the walk instead of NULL. That's the desired
behavior for when it is used from m_start() and m_next() but not so
much in show_smaps_rollup().
I think the cleanest way is to change proc_get_vma() to return NULL
and move the part that deals with gate_vma directly into
m_start()/mm_next(). I'll post it that way and see what others think.
>
> --
> Cheers,
>
> David