On Tue, 2019-12-17 at 18:24 +0100, David Hildenbrand wrote: > >>> Also there are some scenarios where __page_to_pfn is not that simple a > > > > call with us having to get the node ID so we can find the pgdat structure > > > > to perform the calculation. I'm not sure the compiler would be ble to > > > > figure out that the result is the same for both calls, so it is better to > > > > make it explicit. > > > > > > Only in case of CONFIG_SPARSEMEM we have to go via the section - but I > > > doubt this is really worth optimizing here. > > > > > > But yeah, I'm fine with this change, only "IMHO > > > get_pageblock_migratetype() would be nicer" :) > > > > Aren't most distros running with CONFIG_SPARSEMEM enabled? If that is the > > case why not optimize for it? > > Because I tend to dislike micro-optimizations without performance > numbers for code that is not on a hot path. But I mean in this case, as > you said, you need the pfn either way, so it's completely fine with. > > I do wonder, however, if you should just pass in the migratetype from > the caller. That would be even faster ;) The problem is page isolation. We can end up with a page being moved to an isolate pageblock while we aren't holding the zone lock, and as such we likely need to test it again anyway. So there isn't value in storing and reusing the value for cases like page reporting. In addition, the act of isolating the page can cause the migratetype to change as __isolate_free_page will attempt to change the migratetype to movable if it is one of the standard percpu types and we are pulling at least half a pageblock out. So storing the value before we isolate it would be problematic as well. Undoing page isolation is the exception to the issues pointed out above, but in that case we are overwriting the pageblock migratetype anyway so the cache lines involved should all be warm from having just set the value.
