Re: [PATCH v14 67/70] mm/vmscan: Use vma iterator instead of vm_next

From: Yu Zhao
Date: Mon Sep 12 2022 - 23:40:11 EST


On Mon, Sep 12, 2022 at 3:03 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, 12 Sep 2022 14:01:28 -0600 Yu Zhao <yuzhao@xxxxxxxxxx> wrote:
>
> > The diff between the original patch and this one, in case you prefer to
> > fix it atop rather than amend.
>
> Always...
>
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index a7c5d15c1618..cadcc3290918 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -3776,7 +3776,10 @@ static bool get_next_vma(unsigned long mask, unsigned long size, struct mm_walk
> > VM_WARN_ON_ONCE(mask & size);
> > VM_WARN_ON_ONCE((start & mask) != (*vm_start & mask));
> >
> > - for_each_vma_range(vmi, args->vma, end) {
> > + for_each_vma(vmi, args->vma) {
> > + if (end && end <= args->vma->vm_start)
> > + return false;
> > +
> > if (should_skip_vma(args->vma->vm_start, args->vma->vm_end, args))
> > continue;
>
> Thanks.
>
> I added your signoff so I don't get a nastygram from Stephen in the
> morning. Please send along a suitable brief changelog?

mm/vmscan: use the proper VMA iterator

When get_next_vma() finishes iterating VMAs within a range [start,
end), it expects args->vma to point the first VMA out of that range,
if such a VMA exists. This allows its callers to continue the
iteration with a new range above the previous one, if those callers
choose to.

for_each_vma_range() always sets args->vma to NULL after it's done.
This may mislead those callers to conclude that there are no more
VMAs, and in turn they terminate their iterations prematurely.

This fix replaces for_each_vma_range() with for_each_vma() and
explicitly checks whether the next VMA is still within range, and if
not, returns false to indicate the current range has ended. The
callers may continue with the next range if args->vma is not NULL.