Re: [PATCH 01/22] mm: Add vma_lookup()

[Liam Howlett]

* Davidlohr Bueso <dave@xxxxxxxxxxxx> [210520 23:32]:
> On Mon, 10 May 2021, Liam Howlett wrote:
> 
> > Many places in the kernel use find_vma() to get a vma and then check the
> > start address of the vma to ensure the next vma was not returned.
> > 
> > Other places use the find_vma_intersection() call with add, addr + 1 as
> > the range; looking for just the vma at a specific address.
> > 
> > The third use of find_vma() is by developers who do not know that the
> > function starts searching at the provided address upwards for the next
> > vma.  This results in a bug that is often overlooked for a long time.
> > 
> > Adding the new vma_lookup() function will allow for cleaner code by
> > removing the find_vma() calls which check limits, making
> > find_vma_intersection() calls of a single address to be shorter, and
> > potentially reduce the incorrect uses of find_vma().
> > 
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> > ---
> > include/linux/mm.h | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> > 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 25b9041f9925..5f2a15e702ff 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -2689,6 +2689,24 @@ static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m
> > 	return vma;
> > }
> 
> While at it can we clean up find_vma_intersection? I'm not particularly
> user/fan of checkpatch.pl, but this one is kind of ridiculous.

Agreed.  This addition is worth re-spin.  I will change the comment into
a kernel documentation style comment at the same time.

> 
> Thanks,
> Davidlohr
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c274f75efcf9..16eddedf783f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2678,9 +2678,14 @@ extern struct vm_area_struct * find_vma(struct mm_struct * mm, unsigned long add
>  extern struct vm_area_struct * find_vma_prev(struct mm_struct * mm, unsigned long addr,
> 					     struct vm_area_struct **pprev);
> 
> -/* Look up the first VMA which intersects the interval start_addr..end_addr-1,
> -   NULL if none.  Assume start_addr < end_addr. */
> -static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * mm, unsigned long start_addr, unsigned long end_addr)
> +/*
> + * Look up the first VMA which intersects the interval start_addr..end_addr-1,
> + * NULL if none.  Assume start_addr < end_addr.
> + */
> +static inline
> +struct vm_area_struct *find_vma_intersection(struct mm_struct * mm,
> +                                            unsigned long start_addr,
> +                                            unsigned long end_addr)
>  {
> 	struct vm_area_struct * vma = find_vma(mm,start_addr);