| Message ID | 20220906024401.133336-1-chenwandun@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: avoid unnecessary page table walk for __get_user_pages | expand |
On Tue, 6 Sep 2022 10:44:01 +0800 Chen Wandun <chenwandun@huawei.com> wrote: > It is no need to walk page table and find pages if faultin_page success > and __get_user_pages does't care pages at all, so directly handle next > page. > > ... > > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1198,7 +1198,10 @@ static long __get_user_pages(struct mm_struct *mm, > PTR_ERR(page) == -EMLINK, locked); > switch (ret) { > case 0: > - goto retry; > + if (pages) > + goto retry; > + else > + goto next_page; > case -EBUSY: > case -EAGAIN: > ret = 0; Looks right to me. I redid the changelog somewhat: : There is no need to walk page tables and find pages if faultin_page)_ : succeeds and __get_user_pages)_ doesn't care about populating the pages[] : array. So directly go on to handle the next page.
On 9/7/22 16:34, Andrew Morton wrote: > On Tue, 6 Sep 2022 10:44:01 +0800 Chen Wandun <chenwandun@huawei.com> wrote: > >> It is no need to walk page table and find pages if faultin_page success >> and __get_user_pages does't care pages at all, so directly handle next >> page. >> >> ... >> >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -1198,7 +1198,10 @@ static long __get_user_pages(struct mm_struct *mm, >> PTR_ERR(page) == -EMLINK, locked); >> switch (ret) { >> case 0: >> - goto retry; >> + if (pages) >> + goto retry; >> + else >> + goto next_page; Wow, good point. And it's been doing that for a *long* time! >> case -EBUSY: >> case -EAGAIN: >> ret = 0; > > Looks right to me. I redid the changelog somewhat: > > : There is no need to walk page tables and find pages if faultin_page)_ > : succeeds and __get_user_pages)_ doesn't care about populating the pages[] > : array. So directly go on to handle the next page. > > With that re-worded commit description, please feel to add Reviewed-by: John Hubbard <jhubbard@nvidia.com> thanks,
diff --git a/mm/gup.c b/mm/gup.c index 983e24fd4b70..c8de33cc37af 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1198,7 +1198,10 @@ static long __get_user_pages(struct mm_struct *mm, PTR_ERR(page) == -EMLINK, locked); switch (ret) { case 0: - goto retry; + if (pages) + goto retry; + else + goto next_page; case -EBUSY: case -EAGAIN: ret = 0;
It is no need to walk page table and find pages if faultin_page success and __get_user_pages does't care pages at all, so directly handle next page. Signed-off-by: Chen Wandun <chenwandun@huawei.com> --- mm/gup.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)