在 2022/5/21 18:13, Alex Shi 写道:
On 5/21/22 15:23, Chen Wandun wrote:
在 2022/5/19 14:15, Alex Shi 写道:
On 5/19/22 05:38, Suren Baghdasaryan wrote:
On Wed, May 18, 2022 at 3:29 AM Alex Shi <seakeel@xxxxxxxxx> wrote:
On 5/17/22 20:46, Chen Wandun wrote:
This breaks the old ABI. And why you need this new function?
Both great points.
BTW, I think the additional max_threshold parameter could be
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;
}
OK
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.
In fact, I am not keen for this solution, the older and newer
interface is easy to be confused by users, for some resolvable
unclear issues. It's not a good idea.
Maybe adding the max_threshold as an optional last argument will be
less confusing? Smth like this:
some/full min_threshold window_size [max_threshold]
It's already confused enough. :)
which point make you confused？
Interface suggest by Suren is compatible with current version,
I think it is more reasonable and there is no difficuty to understand it.
Your 3rd parameter has different meaning depends on the exists or non-exist
4th one. It's not a good design.
some/full min_threshold window_size [max_threshold]
In this format, the meaning of 3rd parameter keep unchanged.
This format is compatible with current version.
BTW, I still don't see the strong reason for the pressure range.
Considering this case:
I divide pressure into multi levels, and each level corresponds to a
hander, I have to register multi triggers and wait for fire events,
nowadays, these trigger is something like:
echo “some 150000 1000000” > /proc/pressure/memory
echo “some 350000 1000000” > /proc/pressure/memory
echo “some 550000 1000000” > /proc/pressure/memory
echo “some 750000 1000000” > /proc/pressure/memory
In the best case, stall pressure between 150000 and 350000,
only one trigger fire, and only one wakeup.
In any other case, multi triggers fire and multi wakeup, but it
indeed is no need.
Could you give more details info to show what detailed problem
which your propose could address, but current code cannot?
Current code also can handle this, but thing become complex，jsut
as explained above.
Thanks
Wandun
Thanks
Alex
New implement make the fire and wakeup more precise,
userspace code will be more simple, no confusing fire event,
no need to filter fire event anymore, maybe minor performance
improved.
Thanks.
Also, if we do decide to add it, there should be a warning in the
documentation 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.
Right.
And what if others propose more pressure combinations?
Maybe leave them to user space is more likely workable?
Thanks
Alex
.
.
