Re: [PATCH v3 5/7] proc/task_mmu: change proc_get_vma() to stop returning gate VMA at the end

[Suren Baghdasaryan]

On Fri, Sep 11, 2026 at 12:03 PM Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx> wrote:
>
> On Fri, Sep 11, 2026 at 07:26:49PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Sep 10, 2026 at 04:47:35PM -0700, Suren Baghdasaryan wrote:
> > > proc_get_vma() returning gate VMA at the end is desirable for the its
> > > current m_start/m_next callers, as they need to report a gate VMA at the
> > > end of the address space. This behavior is very specific to these callers
> > > and makes proc_get_vma() hard to use for other purposes.
> > >
> > > Move this usage-specific behavior into the callers themselves so that
> > > proc_get_vma() returns either a valid VMA, an error or a NULL when no
> > > more VMAs are available. This makes it more generic, simpler and usable
> > > in the later patches.
> > >
> > > Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> >
> > Yes, very good change, thanks!
> >
> > Reviewed-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> >
> > > ---
> > >  fs/proc/task_mmu.c | 23 ++++++++++++++++++-----
> > >  1 file changed, 18 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > > index ecce7ce116cb..9a3c996c1d61 100644
> > > --- a/fs/proc/task_mmu.c
> > > +++ b/fs/proc/task_mmu.c
> > > @@ -236,9 +236,6 @@ static struct vm_area_struct *proc_get_vma(struct seq_file *m, loff_t *ppos)
> > >              * found the extended vma with the same vm_start.
> > >              */
> > >             *ppos = vma->vm_end;
> > > -   } else {
> > > -           *ppos = SENTINEL_VMA_GATE;
> > > -           vma = get_gate_vma(priv->lock_ctx.mm);
> >
> > Yeah this is just so confusing as-was.
> >
> > >     }
> > >
> > >     return vma;
> > > @@ -248,6 +245,7 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
> > >  {
> > >     struct proc_maps_private *priv = m->private;
> > >     struct proc_maps_locking_ctx *lock_ctx;
> > > +   struct vm_area_struct *vma;
> > >     loff_t last_addr = *ppos;
> > >     struct mm_struct *mm;
> > >
> > > @@ -280,16 +278,31 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
> > >     if (last_addr == SENTINEL_VMA_GATE)
> > >             return get_gate_vma(mm);
> > >
> > > -   return proc_get_vma(m, ppos);
> > > +   vma = proc_get_vma(m, ppos);
> > > +   if (vma)
> > > +           return vma;
> > > +
> > > +   /* Return gate VMA at the end */
> > > +   *ppos = SENTINEL_VMA_GATE;
> > > +   return get_gate_vma(mm);
> > >  }
> > >
> > >  static void *m_next(struct seq_file *m, void *v, loff_t *ppos)
> > >  {
> > > +   struct proc_maps_private *priv = m->private;
> > > +   struct vm_area_struct *vma;
> > > +
> > >     if (*ppos == SENTINEL_VMA_GATE) {
> > >             *ppos = SENTINEL_VMA_END;
> > >             return NULL;
> > >     }
> > > -   return proc_get_vma(m, ppos);
> > > +   vma = proc_get_vma(m, ppos);
> > > +   if (vma)
> > > +           return vma;
> >
> > OK so I guess the logic is, iterate through every VMA, then once you run out,
> > report the gate VMA. Makes sense.
>
> Hmm one thing on this though - m_start() still has:
>
>         if (last_addr == SENTINEL_VMA_GATE)
>                 return get_gate_vma(mm);
>
> As well as:
>
>         vma = proc_get_vma(m, ppos);
>         if (vma)
>                 return vma;
>
>         /* Return gate VMA at the end */
>         *ppos = SENTINEL_VMA_GATE;
>         return get_gate_vma(mm);
>
> Now at the end.
>
> Is this correct? Is it maybe duplicated now?

Yeah, I noticed that too but I it's not duplication. The first check
handles the case when right after m_next() hit the end of the address
space and set *ppos = SENTINEL_VMA_GATE, we ran out of page space and
had to flush its content. Once that's done, m_start will be called and
last_addr will be set to SENTINEL_VMA_GATE. In that case we should
return get_gate_vma() and avoid calling proc_get_vma(). That's what
the first check for sentinel is doing. The second one handles the case
when m_start() itself readches the end of the address space and has to
return SENTINEL_VMA_GATE.

It's possible this can be refactored a bit and made cleaner but I
would keep that as a separate change.

>
> >
> > > +
> > > +   /* Return gate VMA at the end */
> > > +   *ppos = SENTINEL_VMA_GATE;
> > > +   return get_gate_vma(priv->lock_ctx.mm);
> > >  }
> > >
> > >  static void m_stop(struct seq_file *m, void *v)
> > > --
> > > 2.55.0.1007.g17ff1f9808-goog
> > >
> >
> > --
> > Cheers, Lorenzo
>
> --
> Cheers, Lorenzo