On 9/3/20 8:23 PM, Pavel Tatashin wrote: >> >> As expressed in reply to v2, I dislike this hack. There is strong >> synchronization, just PCP is special. Allocating from MIGRATE_ISOLATE is >> just plain ugly. >> >> Can't we temporarily disable PCP (while some pageblock in the zone is >> isolated, which we know e.g., due to the counter), so no new pages get >> put into PCP lists after draining, and re-enable after no pageblocks are >> isolated again? We keep draining the PCP, so it doesn't seem to be of a >> lot of use during that period, no? It's a performance hit already. >> >> Then, we would only need exactly one drain. And we would only have to >> check on the free path whether PCP is temporarily disabled. > > Hm, we could use a static branches to disable it, that would keep > release code just as fast, but I am worried it will make code even > uglier. Let's see what others in this thread think about this idea. Maybe we could just set pcp->high = 0 or something, make sure the pcplist user only reads this value while irqs are disabled. Then the the IPI in drain_all_pages() should guarantee there's nobody racing freeing to pcplist. But careful to not introduce bugs like the one fixed with [1]. And not sure if this guarantee survives when RT comes and replaces the disabled irq's with local_lock or something. [1] https://lore.kernel.org/linux-mm/1597150703-19003-1-git-send-email-charante@xxxxxxxxxxxxxx/ > Thank you, > Pasha >
