Messages in this thread | | | Subject | Re: [PATCH v21 15/19] mm/compaction: do page isolation first in compaction | From | Vlastimil Babka <> | Date | Thu, 12 Nov 2020 12:25:26 +0100 |
| |
On 11/12/20 3:28 AM, Hugh Dickins wrote: > On Wed, 11 Nov 2020, Vlastimil Babka wrote: >> On 11/5/20 9:55 AM, Alex Shi wrote: >> >> > @@ -979,10 +995,6 @@ static bool too_many_isolated(pg_data_t *pgdat) >> > goto isolate_abort; >> > } >> > - /* Recheck PageLRU and PageCompound under lock */ >> > - if (!PageLRU(page)) >> > - goto isolate_fail; >> > - >> > /* >> > * Page become compound since the non-locked check, >> > * and it's on LRU. It can only be a THP so the order >> > @@ -990,16 +1002,13 @@ static bool too_many_isolated(pg_data_t *pgdat) > > Completely off-topic, and won't matter at all when Andrew rediffs into > mmotm: but isn't it weird that this is showing "too_many_isolated(", > when actually the function is isolate_migratepages_block()? > >> > */ >> > if (unlikely(PageCompound(page) && >> > !cc->alloc_contig)) { >> > low_pfn += compound_nr(page) - 1; >> > - goto isolate_fail; >> > + SetPageLRU(page); >> > + goto isolate_fail_put; >> > } >> >> IIUC the danger here is khugepaged will collapse a THP. For that, >> __collapse_huge_page_isolate() has to succeed isolate_lru_page(). Under the >> new scheme, it shouldn't be possible, right? If that's correct, we can remove >> this part? > > I don't think so. A preliminary check for PageCompound was made much > higher up, before taking a reference on the page, but it can easily have > become PageCompound since then (when racing prep_new_page() calls > prep_compound_page()). > > And __collapse_huge_page_isolate() does not turn a non-compound page > into a compound page: it isolates small pages before copying them into > the compound page (in the usual case: I can see there's also allowance > for PageCompound there too, which will do something different).
Right, on both points, got too confused.
> Hugh >
| |