Re: [PATCH v4 14/66] proc: Remove VMA rbtree use from nommu

[Vlastimil Babka <vbabka@suse.cz>]

On 12/1/21 15:29, Liam Howlett wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
> 
> These users of the rbtree should probably have been walks of the
> linked list, but convert them to use walks of the maple tree.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  fs/proc/task_nommu.c | 45 +++++++++++++++++++++-----------------------
>  1 file changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
> index a6d21fc0033c..2fd06f52b6a4 100644
> --- a/fs/proc/task_nommu.c
> +++ b/fs/proc/task_nommu.c
> @@ -20,15 +20,13 @@
>   */
>  void task_mem(struct seq_file *m, struct mm_struct *mm)
>  {
> +	VMA_ITERATOR(vmi, mm, 0);
>  	struct vm_area_struct *vma;
>  	struct vm_region *region;
> -	struct rb_node *p;
>  	unsigned long bytes = 0, sbytes = 0, slack = 0, size;
> -        
> -	mmap_read_lock(mm);
> -	for (p = rb_first(&mm->mm_rb); p; p = rb_next(p)) {
> -		vma = rb_entry(p, struct vm_area_struct, vm_rb);
>  
> +	mmap_read_lock(mm);
> +	for_each_vma(vmi, vma) {
>  		bytes += kobjsize(vma);
>  
>  		region = vma->vm_region;
> @@ -82,15 +80,13 @@ void task_mem(struct seq_file *m, struct mm_struct *mm)
>  
>  unsigned long task_vsize(struct mm_struct *mm)
>  {
> +	VMA_ITERATOR(vmi, mm, 0);
>  	struct vm_area_struct *vma;
> -	struct rb_node *p;
>  	unsigned long vsize = 0;
>  
>  	mmap_read_lock(mm);
> -	for (p = rb_first(&mm->mm_rb); p; p = rb_next(p)) {
> -		vma = rb_entry(p, struct vm_area_struct, vm_rb);
> +	for_each_vma(vmi, vma)
>  		vsize += vma->vm_end - vma->vm_start;
> -	}
>  	mmap_read_unlock(mm);
>  	return vsize;
>  }
> @@ -99,14 +95,13 @@ unsigned long task_statm(struct mm_struct *mm,
>  			 unsigned long *shared, unsigned long *text,
>  			 unsigned long *data, unsigned long *resident)
>  {
> +	VMA_ITERATOR(vmi, mm, 0);
>  	struct vm_area_struct *vma;
>  	struct vm_region *region;
> -	struct rb_node *p;
>  	unsigned long size = kobjsize(mm);
>  
>  	mmap_read_lock(mm);
> -	for (p = rb_first(&mm->mm_rb); p; p = rb_next(p)) {
> -		vma = rb_entry(p, struct vm_area_struct, vm_rb);
> +	for_each_vma(vmi, vma) {
>  		size += kobjsize(vma);
>  		region = vma->vm_region;
>  		if (region) {
> @@ -190,17 +185,19 @@ static int nommu_vma_show(struct seq_file *m, struct vm_area_struct *vma)
>   */
>  static int show_map(struct seq_file *m, void *_p)
>  {
> -	struct rb_node *p = _p;
> -
> -	return nommu_vma_show(m, rb_entry(p, struct vm_area_struct, vm_rb));
> +	return nommu_vma_show(m, _p);
>  }
>  
>  static void *m_start(struct seq_file *m, loff_t *pos)
>  {
>  	struct proc_maps_private *priv = m->private;
>  	struct mm_struct *mm;
> -	struct rb_node *p;
> -	loff_t n = *pos;
> +	struct vm_area_struct *vma;
> +	unsigned long addr = *pos;
> +
> +	/* See m_next(). Zero at the start or after lseek. */
> +	if (addr == -1UL)
> +		return NULL;
>  
>  	/* pin the task and mm whilst we play with them */
>  	priv->task = get_proc_task(priv->inode);
> @@ -216,10 +213,10 @@ static void *m_start(struct seq_file *m, loff_t *pos)
>  		return ERR_PTR(-EINTR);
>  	}
>  
> -	/* start from the Nth VMA */
> -	for (p = rb_first(&mm->mm_rb); p; p = rb_next(p))
> -		if (n-- == 0)
> -			return p;
> +	/* start the next element from addr */
> +	vma = find_vma(mm, addr);
> +	if (vma)
> +		return vma;
>  
>  	mmap_read_unlock(mm);
>  	mmput(mm);
> @@ -242,10 +239,10 @@ static void m_stop(struct seq_file *m, void *_vml)
>  
>  static void *m_next(struct seq_file *m, void *_p, loff_t *pos)
>  {
> -	struct rb_node *p = _p;
> +	struct vm_area_struct *vma = _p;
>  
> -	(*pos)++;
> -	return p ? rb_next(p) : NULL;
> +	*pos = vma->vm_end;
> +	return find_vma(vma->vm_mm, vma->vm_end);
>  }
>  
>  static const struct seq_operations proc_pid_maps_ops = {