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

Liam Howlett
2021-05-21T14:25:22Z

* Davidlohr Bueso <dave@stgolabs.net> [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@Oracle.com>
> > ---
> > 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);