lkml.org 
[lkml]   [2022]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/2] psi: add support for multi level pressure stall trigger
From


在 2022/5/18 1:35, Suren Baghdasaryan 写道:
> On Tue, May 17, 2022 at 5:46 AM Chen Wandun <chenwandun@huawei.com> wrote:
>>
>>
>> 在 2022/5/16 16:43, Suren Baghdasaryan 写道:
>>> On Mon, May 16, 2022 at 1:21 AM Suren Baghdasaryan <surenb@google.com> wrote:
>>>> On Sun, May 15, 2022 at 11:20 PM Alex Shi <seakeel@gmail.com> wrote:
>>>>>
>>>>> On 5/16/22 11:35, Chen Wandun wrote:
>>>>>> Nowadays, psi events are triggered when stall time exceed
>>>>>> stall threshold, but no any different between these events.
>>>>>>
>>>>>> Actually, events can be divide into multi level, each level
>>>>>> represent a different stall pressure, that is help to identify
>>>>>> pressure information more accurately.
>>>> IIUC by defining min and max, you want the trigger to activate when
>>>> the stall is between min and max thresholds. But I don't see why you
>>>> would need that. If you want to have several levels, you can create
>>>> multiple triggers and monitor them separately. For your example, that
>>>> would be:
>>>>
>>>> echo "some 150000 1000000" > /proc/pressure/memory
>>>> echo "some 350000 1000000" > /proc/pressure/memory
>>>>
>>>> Your first trigger will fire whenever the stall exceeds 150ms within
>>>> each 1sec and the second one will trigger when it exceeds 350ms. It is
>>>> true that if the stall jumps sharply above 350ms, you would get both
>>>> triggers firing. I'm guessing that's why you want this functionality
>>>> so that 150ms trigger does not fire when 350ms one is firing but why
>>>> is that a problem? Can't userspace pick the highest level one and
>>>> ignore all the lower ones when this happens? Or are you addressing
>>>> some other requirement?
>>>>
>>>>>> echo "some 150000 350000 1000000" > /proc/pressure/memory would
>>>>> 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.
> Ok, so the issue can be dealt with in the userspace but would make it
> simpler if max_threashold is supported by the kernel. I can buy this
> argument if the kernel implementation is not complex and max_threshold
> is added in a way that does not break current users. I believe both
> conditions can be met.
OK, I will send v2

Thanks
>
>>>>> Thanks
>>>>>
>>>>>> add [150ms, 350ms) threshold for partial memory stall measured
>>>>>> within 1sec time window.
>>>>>>
>>>>>> Signed-off-by: Chen Wandun <chenwandun@huawei.com>
>>>>>> ---
>>>>>> include/linux/psi_types.h | 3 ++-
>>>>>> kernel/sched/psi.c | 19 +++++++++++++------
>>>>>> 2 files changed, 15 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/psi_types.h b/include/linux/psi_types.h
>>>>>> index c7fe7c089718..2b1393c8bf90 100644
>>>>>> --- a/include/linux/psi_types.h
>>>>>> +++ b/include/linux/psi_types.h
>>>>>> @@ -119,7 +119,8 @@ struct psi_trigger {
>>>>>> enum psi_states state;
>>>>>>
>>>>>> /* User-spacified threshold in ns */
>>>>>> - u64 threshold;
>>>>>> + u64 min_threshold;
>>>>>> + u64 max_threshold;
>>>>>>
>>>>>> /* List node inside triggers list */
>>>>>> struct list_head node;
>>>>>> diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
>>>>>> index 6f9533c95b0a..17dd233b533a 100644
>>>>>> --- a/kernel/sched/psi.c
>>>>>> +++ b/kernel/sched/psi.c
>>>>>> @@ -541,7 +541,7 @@ static u64 update_triggers(struct psi_group *group, u64 now)
>>>>>>
>>>>>> /* Calculate growth since last update */
>>>>>> growth = window_update(&t->win, now, total[t->state]);
>>>>>> - if (growth < t->threshold)
>>>>>> + if (growth < t->min_threshold || growth >= t->max_threshold)
>>>>>> continue;
>>>>>>
>>>>>> t->pending_event = true;
>>>>>> @@ -1087,15 +1087,18 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group,
>>>>>> {
>>>>>> struct psi_trigger *t;
>>>>>> enum psi_states state;
>>>>>> - u32 threshold_us;
>>>>>> + u32 min_threshold_us;
>>>>>> + u32 max_threshold_us;
>>>>>> u32 window_us;
>>>>>>
>>>>>> if (static_branch_likely(&psi_disabled))
>>>>>> return ERR_PTR(-EOPNOTSUPP);
>>>>>>
>>>>>> - if (sscanf(buf, "some %u %u", &threshold_us, &window_us) == 2)
>>>>>> + if (sscanf(buf, "some %u %u %u", &min_threshold_us,
>>>>>> + &max_threshold_us, &window_us) == 3)
>>>>>> state = PSI_IO_SOME + res * 2;
>>>>>> - else if (sscanf(buf, "full %u %u", &threshold_us, &window_us) == 2)
>>>>>> + else if (sscanf(buf, "full %u %u %u", &min_threshold_us,
>>>>>> + &max_threshold_us, &window_us) == 3)
>>>>>> state = PSI_IO_FULL + res * 2;
>>>>>> else
>>>>>> return ERR_PTR(-EINVAL);
>>>>>> @@ -1107,8 +1110,11 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group,
>>>>>> window_us > WINDOW_MAX_US)
>>>>>> return ERR_PTR(-EINVAL);
>>>>>>
>>>>>> + if (min_threshold_us >= max_threshold_us)
>>>>>> + return ERR_PTR(-EINVAL);
>>>>>> +
>>>>>> /* Check threshold */
>>>>>> - if (threshold_us == 0 || threshold_us > window_us)
>>>>>> + if (max_threshold_us > window_us)
>>>>>> return ERR_PTR(-EINVAL);
>>>>>>
>>>>>> t = kmalloc(sizeof(*t), GFP_KERNEL);
>>>>>> @@ -1117,7 +1123,8 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group,
>>>>>>
>>>>>> t->group = group;
>>>>>> t->state = state;
>>>>>> - t->threshold = threshold_us * NSEC_PER_USEC;
>>>>>> + t->min_threshold = min_threshold_us * NSEC_PER_USEC;
>>>>>> + t->max_threshold = max_threshold_us * NSEC_PER_USEC;
>>>>>> t->win.size = window_us * NSEC_PER_USEC;
>>>>>> window_reset(&t->win, 0, 0, 0);
>>>>>>
>>> .
> .

\
 
 \ /
  Last update: 2022-05-18 12:00    [W:0.135 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site