Re: [PATCH 3/3] mm/vmscan: replace implicit RECLAIM_ZONE checks with explicit checks

[Dave Hansen]

On 7/1/20 1:04 PM, Ben Widawsky wrote:
>> +static inline bool node_reclaim_enabled(void)
>> +{
>> +	/* Is any node_reclaim_mode bit set? */
>> +	return node_reclaim_mode & (RECLAIM_ZONE|RECLAIM_WRITE|RECLAIM_UNMAP);
>> +}
>> +
>>  extern void check_move_unevictable_pages(struct pagevec *pvec);
>>  
>>  extern int kswapd_run(int nid);
> If a user writes a bit that isn't a RECLAIM_* bit to vm.zone_reclaim_mode 
> today, it acts as though RECLAIM_ZONE is enabled: we try to reclaim in 
> zonelist order before falling back to the next zone in the page allocator.  
> The sysctl doesn't enforce any max value :/  I dont know if there is any 
> such user, but this would break them if there is.
> 
> Should this simply be return !!node_reclaim_mode?

You're right that there _could_ be a user-visible behavior change here.
 But, if there were a change it would be for a bit which wasn't even
mentioned in the documentation.  Somebody would have had to look at the
doc mentioning 1,2,4 and written an 8.  If they did that, they're asking
for trouble because we could have defined the '8' bit to do nasty things
like auto-demote all your memory. :)

I'll mention it in the changelog, but I still think we should check the
actual, known bits rather than check for 0.

BTW, in the hardware, they almost invariably make unused bits "reserved"
and do mean things like #GP if someone tries to set them.  This is a
case where the kernel probably should have done the same.  It would have
saved us the trouble of asking these questions now.  Maybe we should
even do that going forward.