lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZmKVjmuC4kGrUH5V@chenyu5-mobl2>
Date: Fri, 7 Jun 2024 13:07:26 +0800
From: Chen Yu <yu.c.chen@...el.com>
To: Chunxin Zang <spring.cxz@...il.com>
CC: <mingo@...hat.com>, <peterz@...radead.org>, <juri.lelli@...hat.com>,
	<vincent.guittot@...aro.org>, <dietmar.eggemann@....com>,
	<rostedt@...dmis.org>, <bsegall@...gle.com>, <mgorman@...e.de>,
	<bristot@...hat.com>, <vschneid@...hat.com>, <jameshongleiwang@....com>,
	<efault@....de>, <kprateek.nayak@....com>, <linux-kernel@...r.kernel.org>,
	<yangchen11@...iang.com>, <zhouchunhua@...iang.com>,
	<zangchunxin@...iang.com>
Subject: Re: [PATCH v2] sched/fair: Reschedule the cfs_rq when current is
 ineligible

On 2024-05-29 at 22:18:06 +0800, Chunxin Zang wrote:
> I found that some tasks have been running for a long enough time and
> have become illegal, but they are still not releasing the CPU. This
> will increase the scheduling delay of other processes. Therefore, I
> tried checking the current process in wakeup_preempt and entity_tick,
> and if it is illegal, reschedule that cfs queue.
> 
> When RUN_TO_PARITY is enabled, its behavior essentially remains
> consistent with the original process. When NO_RUN_TO_PARITY is enabled,
> some additional preemptions will be introduced, but not too many.
> 
> I have pasted some test results below.
> I isolated four cores for testing and ran hackbench in the background,
> and observed the test results of cyclictest.
> 
> hackbench -g 4 -l 100000000 &
> cyclictest --mlockall -D 5m -q
> 
>                                  EEVDF      PATCH  EEVDF-NO_PARITY  PATCH-NO_PARITY
> 
>                 # Min Latencies: 00006      00006      00006      00006
>   LNICE(-19)    # Avg Latencies: 00191      00133      00089      00066
>                 # Max Latencies: 15442      08466      14133      07713
> 
>                 # Min Latencies: 00006      00010      00006      00006
>   LNICE(0)      # Avg Latencies: 00466      00326      00289      00257
>                 # Max Latencies: 38917      13945      32665      17710
> 
>                 # Min Latencies: 00019      00053      00010      00013
>   LNICE(19)     # Avg Latencies: 37151      25852      18293      23035
>                 # Max Latencies: 2688299    4643635    426196     425708
> 
> I captured and compared the number of preempt occurrences in wakeup_preempt
> to see if it introduced any additional overhead.
> 
> Similarly, hackbench is used to stress the utilization of four cores to
> 100%, and the method for capturing the number of PREEMPT occurrences is
> referenced from [1].
> 
> schedstats                          EEVDF       PATCH   EEVDF-NO_PARITY  PATCH-NO_PARITY  CFS(6.5)
> .stats.check_preempt_count          5053054     5045388    5018589    5029585
> .stats.patch_preempt_count          -------     0020495    -------    0700670    -------
> .stats.need_preempt_count           0570520     0458947    3380513    3116966    1140821
> 
> From the above test results, there is a slight increase in the number of
> preempt occurrences in wakeup_preempt. However, the results vary with each
> test, and sometimes the difference is not that significant.
> 
> [1]: https://lore.kernel.org/all/20230816134059.GC982867@hirez.programming.kicks-ass.net/T/#m52057282ceb6203318be1ce9f835363de3bef5cb
> 
> Signed-off-by: Chunxin Zang <zangchunxin@...iang.com>
> Reviewed-by: Chen Yang <yangchen11@...iang.com>
> 
> ------
> Changes in v2:
> - Make the logic that determines the current process as ineligible and
>   triggers preemption effective only when NO_RUN_TO_PARITY is enabled.
> - Update the commit message
> ---
>  kernel/sched/fair.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 03be0d1330a6..fa2c512139e5 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -745,6 +745,17 @@ int entity_eligible(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  	return vruntime_eligible(cfs_rq, se->vruntime);
>  }
>  
> +static bool check_entity_need_preempt(struct cfs_rq *cfs_rq, struct sched_entity *se)
> +{
> +	if (sched_feat(RUN_TO_PARITY) && se->vlag != se->deadline)
> +		return true;

If I understand correctly, here it intends to check if the current se
has consumed its 1st slice after been picked at set_next_entity(), and if yes do a reschedule.
check_entity_need_preempt() is added at the end of entity_tick(), which could overwrite
the police to reschedule current: (entity_tick()->update_curr()->update_deadline()), only there
are more than 1 runnable tasks will the current be preempted, even if it has expired the 1st
requested slice.

> +
> +	if (!sched_feat(RUN_TO_PARITY) && !entity_eligible(cfs_rq, se))
> +		return true;
> +
> +	return false;
> +}
> +
>  static u64 __update_min_vruntime(struct cfs_rq *cfs_rq, u64 vruntime)
>  {
>  	u64 min_vruntime = cfs_rq->min_vruntime;
> @@ -5523,6 +5534,9 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)
>  			hrtimer_active(&rq_of(cfs_rq)->hrtick_timer))
>  		return;
>  #endif
> +
> +	if (check_entity_need_preempt(cfs_rq, curr))
> +		resched_curr(rq_of(cfs_rq));
>  }
>  
>  
> @@ -8343,6 +8357,9 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>  	cfs_rq = cfs_rq_of(se);
>  	update_curr(cfs_rq);
>  
> +	if (check_entity_need_preempt(cfs_rq, se))
> +		goto preempt;
> +

As we changes the preemption policy for current in two places, the tick preemption and wakeup preemption,
do you have statistics that shows which one brings the most benefit?

thanks,
Chenyu

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ