On 2/26/25 16:57, Baoquan He wrote: > On 02/26/25 at 01:01pm, Michal Hocko wrote: >> >> Why do you think anything needs to be adjusted? > > No, I don't think like that. But I am wondering what makes you get > the conclusion. > >> >> > I haven't thought of the whole zone fallback list to interleave nodes >> > which invovles a lot of change. >> > >> > > >> > > Btw. has 96a5c186efff tried to fix any actual runtime problem? The >> > > changelog doesn't say much about that. >> > >> > No, no actual problem was observed on tht. >> >> OK >> >> > I was just trying to make >> > clear the semantics because I was confused by its obscure value printing >> > of zone->lowmem_reserve[] in /proc/zoneinfo. >> > >> > I think we can merge this reverting firstly, then to investigate how to >> > better clarify it. >> >> What do you think needs to be clarify? How exactly is the original >> output confusing? > > When I did the change, I wrote the reason in commit log. I don't think > you care to read it from your talking. Let me explain again, in > setup_per_zone_lowmem_reserve(), each zone's protection value is > calculated based on its own node's zones. E.g below on node 0, its > Movable zone and Device zone are empty but still show influence on > Normal/DMA32/DMA zone, this is unreasonable from the protection value > calculating code and its showing. It's not unreasonable. A GFP_HIGHUSER_MOVABLE can use up to the Movable zone, so e.g. the dma32 zone should be protected from such an allocation, so it has space for GFP_DMA32 restricted allocations. If no Movable zone exists, but Normal zone does, the result is the protection will be the same for GFP_KERNEL allocations (that can use up to the Normal zone) and GFP_HIGHUSER_MOVABLE allocations. (i.e. the number of 22134 in your listing is the same for both indexes). That's fine. But setting the protection from Movable allocations to 0 as commit 96a5c186efff did was simply a bug, as that can directly lead to GFP_HIGHUSER_MOVABLE depleting ZONE_DMA32. The only "unreasonable" part here is that we define and show protections from ZONE_DEVICE allocations. The usage of this zone is AFAIK completely separate from normal page allocation through zonelists, so we could exclude it, if anyone cared enough. > If really as your colleague Gabriel said, the protection value of DMA zone > on node 0 will impact allocation when targeted zone is Movable zone, we > may need consider the protection value calcuation acorss nodes. Because > the impact happens among different nodes. I only said we can do > investigation, I didn't said we need change or have to change. There might be a theoretical issue if e.g. Node 0 only contained DMA and DMA32 zones and nothing else, while the Normal zone is on Node 1, there would be no protection for DMA/DMA32 zones from Normal allocations, as setup_per_zone_lowmem_reserve() considers each node separately and thus would not take Normal zone size from Node 1 into account. Should we sum zone sizes accross all nodes then? But then __GFP_THISNODE Normal allocations for node 0 would never succeed? Or we'd need a separate lowmem_reserve array for those? I guess the issue doesn't happen in practice. In any case it's out of scope of the reverted commit and the revert. > Node 0, zone DMA > ...... > pages free 2816 > ...... > protection: (0, 1582, 23716, 23716, 23716) > Node 0, zone DMA32 > pages free 403269 > ...... > protection: (0, 0, 22134, 22134, 22134) > Node 0, zone Normal > pages free 5423879 > ...... > protection: (0, 0, 0, 0, 0) > Node 0, zone Movable > pages free 0 > ...... > protection: (0, 0, 0, 0, 0) > Node 0, zone Device > pages free 0 > ...... > protection: (0, 0, 0, 0, 0) > >> >> -- >> Michal Hocko >> SUSE Labs >> > >
