Re: [PATCH] mm, vmscan: guarantee drop_slab_node() termination

[Vlastimil Babka <vbabka@suse.cz>]

On 8/19/21 16:16, Michal Hocko wrote:
> On Thu 19-08-21 14:21:08, Chris Down wrote:
>> 
>> Yeah, but we already draw the line at 10 right now. `freed > 10 && retries <
>> MAX_RECLAIM_RETRIES` seems easier to reason about, at least to me, and stays
>> closer to the current behaviour while providing a definitive point of loop
>> termination.

The doubling threshold is also definitive :) 64 iterations on 64-bit systems, 32
on 32 bit. In practice it will be much less.

> I have to say that I am not really a fan of MAX_RECLAIM_RETRIES approach
> especially for user interfaces. Any limit on retries has kicked us back
> (e.g. offlining for the memory hotplug just to mention one of those).
> drop_caches can take a long time on its own even without retrying. We
> should teach people to interrupt those operations if they should really
> finish early (e.g. timeout $TIMEOUT echo > /proc/sys/vm/drop_caches)
> rather than trying to be extra clever here.

I'm afraid the retries vs threshold has more potential for bikeshedding than
actual difference in practice. I can change it if more people prefer that way.

> I am not against the patch Vlastimil is proposing because it replaces an
> ad-hoc limit on the reclaimed objects threshold with something that is
> less "random" - sort of a backoff instead seems like an improvement to
> me. But I would still be worried that this could regress for some users
> so in an ideal world the existing bail on signal should be enough.

My point of view on this is:

- first let's not forget this is an interface intended for debugging, that we
generally discourage to use in a regular manner. Yet it exists, so people will
inevitably use it from automated scripts and cron and whatnot.

- as such there are no guarantees how much it reclaims, it's a best effort
really. If a user thinks it hasn't reclaimed enough, they can always implement
own userspace loop that checks e.g. meminfo or slabinfo to decide whether to do
another iteration of drop_caches.

- while the userspace loop might be convoluted, the idea of running with timeout
is not obvious. One might see the operation finishes fine in initial testing,
put it in cron etc. without timeout, and one day the infinite loop kicks in and
there's trouble.

So I think the safer default that avoids suprises is to guarantee termination
rather than aggressive reclaim.