Re: [PATCH -next] mm/vmscan: fix an undefined behavior for zone id

[Johannes Weiner]

Qian, thanks for the report and the fix.

On Mon, Nov 11, 2019 at 02:28:12PM +0100, Michal Hocko wrote:
> On Mon 11-11-19 13:14:27, Chris Down wrote:
> > Chris Down writes:
> > > Ah, I just saw this in my local checkout and thought it was from my
> > > changes, until I saw it's also on clean mmots checkout. Thanks for the
> > > fixup!
> > 
> > Also, does this mean we should change callers that may pass through
> > zone_idx=MAX_NR_ZONES to become MAX_NR_ZONES-1 in a separate commit, then
> > remove this interim fixup? I'm worried otherwise we might paper over real
> > issues in future.
> 
> Yes, removing this special casing is reasonable. I am not sure
> MAX_NR_ZONES - 1 is a better choice though. It is error prone and
> zone_idx is the highest zone we should consider and MAX_NR_ZONES - 1
> be ZONE_DEVICE if it is configured. But ZONE_DEVICE is really standing
> outside of MM reclaim code AFAIK. It would be probably better to have
> MAX_LRU_ZONE (equal to MOVABLE) and use it instead.

We already use MAX_NR_ZONES - 1 everywhere else in vmscan.c to mean
"no zone restrictions" - get_scan_count() is the odd one out:

- mem_cgroup_shrink_node()
- try_to_free_mem_cgroup_pages()
- balance_pgdat()
- kswapd()
- shrink_all_memory()

It's a little odd that it points to ZONE_DEVICE, but it's MUCH less
subtle than handling both inclusive and exclusive range delimiters.

So I think the better fix would be this:

---