which point make you confused?
On 5/19/22 05:38, Suren Baghdasaryan wrote:
On Wed, May 18, 2022 at 3:29 AM Alex Shi <seakeel@xxxxxxxxx> wrote:It's already confused enough. :)
Maybe adding the max_threshold as an optional last argument will be
On 5/17/22 20:46, Chen Wandun wrote:
In fact, I am not keen for this solution, the older and newerOKBTW, I think the additional max_threshold parameter could beThis breaks the old ABI. And why you need this new function?Both great points.
implemented in a backward compatible way so that the old API is not
broken:
arg_count = sscanf(buf, "some %u %u %u", &min_threshold_us, &arg2, &arg3);
if (arg_count < 2) return ERR_PTR(-EINVAL);
if (arg_count < 3) {
max_threshold_us = INT_MAX;
window_us = arg2;
} else {
max_threshold_us = arg2;
window_us = arg3;
}
Thanks.
But again, the motivation still needs to be explained.we want do different operation for different stall level,
just as prev email explain, multi trigger is also OK in old
ways, but it is a litter complex.
interface is easy to be confused by users, for some resolvable
unclear issues. It's not a good idea.
less confusing? Smth like this:
some/full min_threshold window_size [max_threshold]
BTW, I still don't see the strong reason for the pressure range.Considering this case:
Right.Also, if we do decide to add it, there should be a warning in thedocumentation that max_threshold usage might lead to a stall being
missed completely. In your example:
echo "some 150000 350000 1000000" > /proc/pressure/memory
If there is a stall of more than 350ms within a given window, that
trigger will not fire at all.
And what if others propose more pressure combinations?
Maybe leave them to user space is more likely workable?
Thanks
Alex
.