On Fri Nov 14, 2025 at 10:40 AM UTC, fujunjie wrote:Although this code is not on a hot path, the revised form is clearerIs it...? If people do think it is clearer, let's at least write the right comment in the right place. Instead of having one piece of code (calculate_totalreserve_pages()) describe at a distance the behaviour of another piece of code (setup_per_zone_lowmem_reserve()), let's describe an invariant of the data ("lowmem_reserve is monotonic, up to the first zero value"), at the site where the data is defined. I know sometimes in code this complex we do need these "spooky-action-at-a-distance" comments but this doesn't seem like one of those places to me.
Thanks for the review, Brendan! Let me clarify the motivation using the actual semantics of zone->lowmem_reserve[j], since that meaning is what leads to the monotonic property. For a given zone “i” (i.e., zone_idx(zone) == i), the entry zone->lowmem_reserve[j] (for j > i) represents how many pages in this zone “i” must be treated as reserved when evaluating whether an allocation class that is allowed to use zones up to “j” may fall back into zone “i”. These reserved pages protect more constrained allocations that cannot use higher zones and may rely heavily on this lower zone. Because of this meaning, as j increases we are considering allocation classes that are able to use a strictly larger set of zones. Such classes are more flexible, and therefore we should not allow them to consume more low memory than allocation classes with a smaller j. Consequently, the reserved amount for zone “i” can only stay the same or increase as j increases; it should not decrease. setup_per_zone_lowmem_reserve() encodes this semantics directly: as j increases, it accumulates the managed pages of higher zones and computes zone->lowmem_reserve[j] as managed_pages / ratio[i]. This makes zone->lowmem_reserve[j] monotonically non-decreasing in j by design, reflecting the intended protection model rather than an accidental implementation detail. Given this structure, scanning backward from the highest j and taking the first non-zero entry in calculate_totalreserve_pages() is a natural way to use the data: the maximum reserve for zone “i” must appear at the highest j for which the reserve is defined. For readers familiar with how zone->lowmem_reserve[j] is constructed, encountering a full forward maximum scan can raise the question “why search the entire range when the reserve grows monotonically with j?” That said, I agree with your point that if users rely on this property, the monotonicity should be documented where zone->lowmem_reserve[j] is populated (or near the field definition), rather than explained indirectly from calculate_totalreserve_pages(). I can move the comment there and keep the consuming code simple. I’ll wait a bit to see if others have opinions, and if the direction seems agreeable I can send a v2 with the comment relocated accordingly. Thanks again for the feedback!