On Tue 13-11-18 01:39:42, Wei Yang wrote: > On Mon, Nov 12, 2018 at 03:40:20PM +0100, Michal Hocko wrote: > >On Mon 12-11-18 14:26:41, Wei Yang wrote: > >> On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote: > >> >On Mon 12-11-18 15:14:04, Wei Yang wrote: > >> >> Zone with no managed_pages doesn't contribute totalreserv_pages. And the > >> >> more nodes we have, the more empty zones there are. > >> >> > >> >> This patch skip the zones to save some cycles. > >> > > >> >What is the motivation for the patch? Does it really cause any > >> >measurable difference in performance? > >> > > >> > >> The motivation here is to reduce some unnecessary work. > > > >I have guessed so even though the changelog was quite modest on the > >motivation. > > > >> Based on my understanding, almost every node has empty zones, since > >> zones within a node are ordered in monotonic increasing memory address. > > > >Yes, this is likely the case. Btw. a check for populated_zone or > >for_each_populated_zone would suite much better. > > > > Hmm... maybe not exact. > > populated_zone checks zone->present_pages > managed_zone checks zone->managed_pages > > As the comment of managed_zone says, this one records the pages managed > by buddy system. And when we look at the usage of totalreserve_pages, it > is only used in page allocation. And finally, *max* is checked with > managed_pages instead of present_pages. > > Because of this, managed_zone is more accurate at this place. Is my > understanding correct? OK, fair enough. There is a certain discrepancy here. You are right that we do not care about pages out of the page allocator scope (e.g. early bootmem allocations, struct pages) but this is likely what other callers of populated_zone are looking for as well. It seems that managed pages counter which only came in later was not considered in other places. That being said this asks for a cleanup of some sort. And I think such a cleanup wold be appreciated much more than an optimization of an unknown effect and wonder why this check is used here and not at other places. -- Michal Hocko SUSE Labs
