Re: [PATCH] mm/vmscan: restore zone_reclaim_mode ABI

[Baoquan He]

On 06/29/20 at 07:27am, Dave Hansen wrote:
> On 6/28/20 11:52 PM, Baoquan He wrote:
> > On 06/25/20 at 05:34pm, Dave Hansen wrote:
> >>
> >> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> >>
> >> I went to go add a new RECLAIM_* mode for the zone_reclaim_mode
> >> sysctl.  Like a good kernel developer, I also went to go update the
> >> documentation.  I noticed that the bits in the documentation didn't
> >> match the bits in the #defines.
> >>
> >> The VM evidently stopped caring about RECLAIM_ZONE at some point (or
> >> never cared) and the #define itself was later removed as a cleanup.
> > 
> >>From git history, it seems to never care about the RECLAIM_ZONE bit.
> > 
> > I think this patch is justified. I have one question about adding back
> > the RECLAIM_ZONE bit. Since we introduced RECLAIM_ZONE in the first
> > place, but never use it, removing it truly may fail some existing
> > script, does it mean we will never have chance to fix/clean up this kind
> > of mess?
> 
> Our #1 rule is "don't break userspace".  We only break userspace when we
> have no other choice.
> 
> This case a bit fuzzier because we don't know if anyone is depending on
> the ignored bit to do anything.  But, it *was* documented.  To me, that
> means it might have been used, even though it would have been a
> "placebo" bit.
> 
> > Do we have possibility to remove it in mainline tree, let distos or
> > stable kernel maintainer take care of the back porting? Like this, any
> > stable kernel after 5.8, or any distrols which chooses post v5.8 kenrel
> > as base won't have this confusion. I am not objecting this patch, just
> > be curious if we have a way to fix/clean up for this type of issue.
> 
> The only way I can plausibly think of "cleaning up" the RECLAIM_ZONE bit
> would be to raise our confidence that it is truly unused.  That takes
> time, and probably a warning if we see it being set.  If we don't run
> into anybody setting it or depending on it being set in a few years, we
> can remove it.

So adding the old bit back for compatibility looks good, thanks.

Then we have to be very careful when adding and reviewing new
interface introducing, should not leave one which might be used
in the future.

In fact, RECLAIM_ZONE is not completely useless. At least, when the old
bit 0 is set, it may enter into node_reclaim() in get_page_from_freelist(),
that makes it like a switch.

get_page_from_freelist {

	...
                        if (node_reclaim_mode == 0 ||                                                                                             
                            !zone_allows_reclaim(ac->preferred_zoneref->zone, zone))
                                continue;
	...
}


> 
> Backporting a _warning_ into the -stable trees might be an interesting
> way to find users of older kernels mucking with it.
>