Messages in this thread | | | Date | Mon, 15 Aug 2022 13:15:55 -0300 | From | Jason Gunthorpe <> | Subject | Re: [PATCH v4 2/2] mm/gup.c: Refactor check_and_migrate_movable_pages() |
| |
On Fri, Aug 12, 2022 at 12:13:09PM +1000, Alistair Popple wrote: > When pinning pages with FOLL_LONGTERM check_and_migrate_movable_pages() > is called to migrate pages out of zones which should not contain any > longterm pinned pages. > > When migration succeeds all pages will have been unpinned so pinning > needs to be retried. Migration can also fail, in which case the pages > will also have been unpinned but the operation should not be retried. If > all pages are in the correct zone nothing will be unpinned and no retry > is required. > > The logic in check_and_migrate_movable_pages() tracks unnecessary state > and the return codes for each case are difficult to follow. Refactor the > code to clean this up. No behaviour change is intended. > > Signed-off-by: Alistair Popple <apopple@nvidia.com> > > ---
This seems like the cleanest version yet!
> +static long check_and_migrate_movable_pages(unsigned long nr_pages, > + struct page **pages) > +{ > + int ret; > + unsigned long collected; > + LIST_HEAD(movable_page_list); > + > + collected = collect_longterm_unpinnable_pages(&movable_page_list, nr_pages, pages); > + if (!collected) > + return 0; > + > + ret = migrate_longterm_unpinnable_pages(&movable_page_list, nr_pages, pages); > + if (!ret) > + return -EAGAIN; > + else > + return ret;
I would drop the else path and just return zero
Arguably migrate_longterm_unpinnable_pages() should do the same?
> @@ -2051,10 +2079,10 @@ static long __gup_longterm_locked(struct mm_struct *mm, > break; > > rc = check_and_migrate_movable_pages(rc, pages); > - } while (!rc); > + } while (rc == -EAGAIN);
Since the only reader only cares about errno or not errno..
But no biggie either way
Thanks, Jason
| |