On Tue, Jan 26, 2021 at 1:49 AM Vlastimil Babka [off-list ref] wrote:

Maybe instead of replacing one magic value of 'pfn' with another
(cc->migrate_pfn with high_pfn) I would just add a "bool found" and use it
appropriately. Would make the function less subtle perhaps.

I agree. Using that bool variable will look good.

While reviewing I found more potentially questionable parts, if you're interested:

- if we go through "if (get_pageblock_skip(freepage))... continue;" we are
increasing nr_scanned++; but not checking the limit.

Yes, you've got a point. I think nr_scanned checking should be first.

- the "if (list_is_last(freelist, &freepage->lru)) break;" part seems
unneccesary? if we are on the last page and just "continue;" the
list_for_each_entry() iteration should stop anyway.

Yes It seems unnecessary. only if the skipped block was reordered, it
would be required.

Then I will reflect your opinion and send a new version.