| Message ID | 20211201142918.921493-51-Liam.Howlett@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introducing the Maple Tree | expand |
On 12/1/21 15:30, Liam Howlett wrote: > From: "Liam R. Howlett" <Liam.Howlett@Oracle.com> Empty message. > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> > --- > mm/gup.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/mm/gup.c b/mm/gup.c > index 2c51e9748a6a..60892e5df6a2 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1570,7 +1570,6 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) > long ret = 0; > > end = start + len; > - > for (nstart = start; nstart < end; nstart = nend) { > /* > * We want to fault in pages for [nstart; end) address range. > @@ -1579,10 +1578,10 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) > if (!locked) { > locked = 1; > mmap_read_lock(mm); > - vma = find_vma(mm, nstart); > + vma = find_vma_intersection(mm, start, end); Is it ok to use start instead of nstart? At least theoretically, populate_vma_page_range() called later can reset locked to 0, so !locked doesn't guarantee we are in the first iteration? > } else if (nstart >= vma->vm_end) > - vma = vma->vm_next; > - if (!vma || vma->vm_start >= end) > + vma = find_vma(mm, vma->vm_end); > + if (!vma) > break; > /* > * Set [nstart; nend) to intersection of desired address
* Vlastimil Babka <vbabka@suse.cz> [220119 12:39]: > On 12/1/21 15:30, Liam Howlett wrote: > > From: "Liam R. Howlett" <Liam.Howlett@Oracle.com> > > Empty message. > > > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> > > --- > > mm/gup.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index 2c51e9748a6a..60892e5df6a2 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -1570,7 +1570,6 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) > > long ret = 0; > > > > end = start + len; > > - > > for (nstart = start; nstart < end; nstart = nend) { > > /* > > * We want to fault in pages for [nstart; end) address range. > > @@ -1579,10 +1578,10 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) > > if (!locked) { > > locked = 1; > > mmap_read_lock(mm); > > - vma = find_vma(mm, nstart); > > + vma = find_vma_intersection(mm, start, end); > > Is it ok to use start instead of nstart? At least theoretically, > populate_vma_page_range() called later can reset locked to 0, so !locked > doesn't guarantee we are in the first iteration? It is not okay to use start instead of nstart, I will fix this patch. > > > } else if (nstart >= vma->vm_end) > > - vma = vma->vm_next; > > - if (!vma || vma->vm_start >= end) > > + vma = find_vma(mm, vma->vm_end); > > + if (!vma) > > break; > > /* > > * Set [nstart; nend) to intersection of desired address >
diff --git a/mm/gup.c b/mm/gup.c index 2c51e9748a6a..60892e5df6a2 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1570,7 +1570,6 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) long ret = 0; end = start + len; - for (nstart = start; nstart < end; nstart = nend) { /* * We want to fault in pages for [nstart; end) address range. @@ -1579,10 +1578,10 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) if (!locked) { locked = 1; mmap_read_lock(mm); - vma = find_vma(mm, nstart); + vma = find_vma_intersection(mm, start, end); } else if (nstart >= vma->vm_end) - vma = vma->vm_next; - if (!vma || vma->vm_start >= end) + vma = find_vma(mm, vma->vm_end); + if (!vma) break; /* * Set [nstart; nend) to intersection of desired address