Re: [PATCH v2 4/5] proc/task_mmu: read proc/pid/smaps_rollup under per-vma lock

[Suren Baghdasaryan]

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.

>
> > +             if (vma == get_gate_vma(lock_ctx->mm))
> > +                     break;
> >
> >               /*
> > -              * Release mmap_lock temporarily if someone wants to
> > -              * access it for write request.
> > +              * If after retaking the lock, already reported VMA grew or
> > +              * merged with the next one, smap_gather_stats() will gather
> > +              * stats for the remaining portion by starting at last_vma_end.
> >                */
> > -             if (mmap_lock_is_contended(mm)) {
> > -                     vma_iter_invalidate(&vmi);
> > -                     unlock_ctx_mm(&priv->lock_ctx);
> > -                     ret = lock_ctx_mm(&priv->lock_ctx);
> > -                     if (ret) {
> > -                             release_task_mempolicy(priv);
> > -                             goto out_put_mm;
> > -                     }
> > +             smap_gather_stats(priv, vma, &mss, last_vma_end);
> > +             last_vma_end = vma->vm_end;
>
> I skimmed over the remaining bits, hoping the VMA lock experts will review in
> detail :)

Thanks! I'll wait another day and if there are no more comments, I
will post the next version.

>
> --
> Cheers,
>
> David
>