On 2/26/20 6:53 PM, Rik van Riel wrote: > On Wed, 2020-02-26 at 10:48 +0100, Vlastimil Babka wrote: >> On 2/25/20 7:44 PM, Rik van Riel wrote: >> >> Uh, is it any different from base pages which have to pass the same >> check? I >> guess the caller could do e.g. lru_add_drain_all() first. > > You are right, it is not different. > > As for lru_add_drain_all(), I wonder at what point that > should happen? Right now it seems to be done in alloc_contig_range(), but rather late. > It appears that the order in which things are done does > not really provide a good moment: > 1) decide to attempt allocating a range of memory > 2) scan each page block for unmovable pages > 3) if no unmovable pages are found, mark the page block > MIGRATE_ISOLATE > > I wonder if we should do things the opposite way, first > marking the page block MIGRATE_ISOLATE (to prevent new > allocations), then scanning it, and calling lru_add_drain_all > if we encounter a page that looks like it could benefit from > that. > > If we still see unmovable pages after that, it is cheap > enough to set the page block back to its previous state. Yeah seems like the whole has_unmovable_pages() thing isn't much useful here. It might prevent some unnecessary action like isolating something, then finding non-movable page and rolling back the isolation. But maybe it's not worth the savings, and also has_unmovable_pages() being false doesn't guarantee succeed in the actual isolate+migrate attempt. And if it can cause a false negative due to lru pages not drained, then it's actually worse than if it wasn't called at all.
