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

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

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@google.com>
>
> Yes, very good change, thanks!
>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>
> > ---
> >  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?

>
> > +
> > +	/* 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