Jason Gunthorpe <jgg@xxxxxxxxxx> writes:
> On Wed, Aug 17, 2022 at 01:35:12PM -0700, John Hubbard wrote:
>> How's this look to you:
I agree, I think all the refactoring left this written in a weird way. I
was going to suggest this though:
collected = collect_longterm_unpinnable_pages(&movable_page_list,
nr_pages, pages);
if (collected == 0)
return 0;
ret = migrate_longterm_unpinnable_pages(&movable_page_list, nr_pages,
pages);
if (ret)
return ret;
return -EAGAIN;
Which IMHO looks at lot more normal and sane than what I had.
>> collected = collect_longterm_unpinnable_pages(&movable_page_list,
>> nr_pages, pages);
>> if (collected == 0)
>> return 0;
>>
>> ret = migrate_longterm_unpinnable_pages(&movable_page_list, nr_pages,
>> pages);
>>
>> /* If we got here, we have some unpinnable pages... */
>>
>> if (ret == 0) {
>> /*
>> * ...and we successfully migrated those pages. Which means that
>> * the caller should retry the operation now.
>> */
>> ret = -EAGAIN;
>
> return -EAGAIN
>
>> }
>>
>> return ret;
>
> But why return 0 from the helper function in the first place?
To stick with the paradigm of 0 == success. Ie.
migrate_longterm_unpinnable_pages() successfully migrated everything
requested. I don't feel particularly strongly about this though - happy
to return -EAGAIN directly from migrate_longterm_unpinnable_pages() and
just pass that return code up the stack if others think it's clearer.
> Jason
