lkml.org 
[lkml]   [2022]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v14 67/70] mm/vmscan: Use vma iterator instead of vm_next
On Tue, Sep 06, 2022 at 07:49:05PM +0000, Liam Howlett wrote:
> Use the vma iterator in in get_next_vma() instead of the linked list.
>
> Suggested-by: Yu Zhao <yuzhao@google.com>

Apologies for the bad suggestion.

> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3776,23 +3776,14 @@ static bool get_next_vma(unsigned long mask, unsigned long size, struct mm_walk
> {
> unsigned long start = round_up(*vm_end, size);
> unsigned long end = (start | ~mask) + 1;
> + VMA_ITERATOR(vmi, args->mm, start);
>
> VM_WARN_ON_ONCE(mask & size);
> VM_WARN_ON_ONCE((start & mask) != (*vm_start & mask));
>
> - while (args->vma) {
> - if (start >= args->vma->vm_end) {
> - args->vma = args->vma->vm_next;
> + for_each_vma_range(vmi, args->vma, end) {
> + if (should_skip_vma(args->vma->vm_start, args->vma->vm_end, args))
> continue;
> - }
> -
> - if (end && end <= args->vma->vm_start)
> - return false;

Here the original code leaves args->vma pointing the first vma out of
the range [start, end). This allows the caller (page table walker) to
resume at that vma, if it chooses to.

With for_each_vma_range(), under the same condition, args->vma is set to
NULL. And the page table walker may terminate prematurely. Apparently I
overlooked until I was told MGLRU in mm-unstable is slower than itself
on 6.0-rc4 yesterday.

> -
> - if (should_skip_vma(args->vma->vm_start, args->vma->vm_end, args)) {
> - args->vma = args->vma->vm_next;
> - continue;
> - }
>
> *vm_start = max(start, args->vma->vm_start);
> *vm_end = min(end - 1, args->vma->vm_end - 1) + 1;

The following should work properly. Please take a look. Thanks!

---
mm/vmscan.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 11a86d47e85e..b22d3efe3031 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3776,23 +3776,17 @@ static bool get_next_vma(unsigned long mask, unsigned long size, struct mm_walk
{
unsigned long start = round_up(*vm_end, size);
unsigned long end = (start | ~mask) + 1;
+ VMA_ITERATOR(vmi, args->mm, start);

VM_WARN_ON_ONCE(mask & size);
VM_WARN_ON_ONCE((start & mask) != (*vm_start & mask));

- while (args->vma) {
- if (start >= args->vma->vm_end) {
- args->vma = args->vma->vm_next;
- continue;
- }
-
+ 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)) {
- args->vma = args->vma->vm_next;
+ if (should_skip_vma(args->vma->vm_start, args->vma->vm_end, args))
continue;
- }

*vm_start = max(start, args->vma->vm_start);
*vm_end = min(end - 1, args->vma->vm_end - 1) + 1;
--
2.37.2.789.g6183377224-goog
\
 
 \ /
  Last update: 2022-09-12 08:56    [W:0.339 / U:24.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site