Re: [PATCH v4 2/2] mm/gup.c: Refactor check_and_migrate_movable_pages()

[Jason Gunthorpe]

On Tue, Aug 16, 2022 at 03:29:20PM +1000, Alistair Popple wrote:

> >> +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
> 
> Unless I've misunderstood you I don't think that's correct though.
> check_and_migrate_movable_pages() needs to return one of 3 conditions:
> 
>  - 0: All pages are in the correct zone and are still pinned (ie. "success").
>  - -EAGAIN: Some pages were migrated, all pages need re-pinning.
>  - errno: Migration failed, pins were dropped and PUP should fail.
> 
> John's suggested comment update spells this out more clearly.

But that is not what it does..

if (!ret) == if (ret != 0)
        return -EAGAIN
thus ret == 0
return ret == return 0

Jason