Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <30b37eeb-e77b-882e-fc24-3367321a8ca3@gmail.com>
Date:   Mon, 16 May 2022 14:20:24 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH 1/2] psi: add support for multi level pressure stall
 trigger
Content-Language: en-US
To:     Chen Wandun <chenwandun@huawei.com>, linux-kernel@vger.kernel.org,
        hannes@cmpxchg.org, surenb@google.com, alexs@kernel.org,
        corbet@lwn.net, linux-doc@vger.kernel.org
References: <20220516033524.3130816-1-chenwandun@huawei.com>
From:   Alex Shi <seakeel@gmail.com>
In-Reply-To: <20220516033524.3130816-1-chenwandun@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


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.
> 
> echo "some 150000 350000 1000000" > /proc/pressure/memory would

This breaks the old ABI. And why you need this new function?

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);
>