[PATCH 1/1] sched/fair:Reduce unnecessary check preempt in the sched tick

[PATCH 1/1] sched/fair:Reduce unnecessary check preempt in the sched tick

[qianjun.kernel]

From: jun qian <qianjun.kernel@gmail.com>

If it has been determined that the current cpu need resched in the
early stage of for_each_sched_entity, then there is no need to check
preempt in the subsequent se->parent entity_tick.

Signed-off-by: jun qian <qianjun.kernel@gmail.com>
---
 kernel/sched/fair.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1a68a0536add..c0d135100d54 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4352,8 +4352,13 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
 {
 	unsigned long ideal_runtime, delta_exec;
 	struct sched_entity *se;
+	struct rq *rq = rq_of(cfs_rq);
 	s64 delta;
 
+	/* If the TIF_NEED_RESCHED has been set, it is no need to check again */
+	if (test_tsk_need_resched(rq->curr))
+		return;
+
 	ideal_runtime = sched_slice(cfs_rq, curr);
 	delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
 	if (delta_exec > ideal_runtime) {
-- 
2.18.2

Re: [PATCH 1/1] sched/fair:Reduce unnecessary check preempt in the sched tick

[Peter Zijlstra]

On Tue, Apr 13, 2021 at 09:18:42PM +0800, qianjun.kernel@gmail.com wrote:
> From: jun qian <qianjun.kernel@gmail.com>
> 
> If it has been determined that the current cpu need resched in the
> early stage of for_each_sched_entity, then there is no need to check
> preempt in the subsequent se->parent entity_tick.

Right, but does it actually do anything, except increase linecount?

> Signed-off-by: jun qian <qianjun.kernel@gmail.com>
> ---
>  kernel/sched/fair.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 1a68a0536add..c0d135100d54 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4352,8 +4352,13 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
>  {
>  	unsigned long ideal_runtime, delta_exec;
>  	struct sched_entity *se;
> +	struct rq *rq = rq_of(cfs_rq);
>  	s64 delta;
>  
> +	/* If the TIF_NEED_RESCHED has been set, it is no need to check again */
> +	if (test_tsk_need_resched(rq->curr))
> +		return;
> +
>  	ideal_runtime = sched_slice(cfs_rq, curr);
>  	delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
>  	if (delta_exec > ideal_runtime) {

Also, I think that's placed wrong; this way we can mis clear_buddies().

Re: [PATCH 1/1] sched/fair:Reduce unnecessary check preempt in the sched tick

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5446 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on v5.12-rc7 next-20210413]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/qianjun-kernel-gmail-com/sched-fair-Reduce-unnecessary-check-preempt-in-the-sched-tick/20210413-212057
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 4aed8aa41524a1fc6439171881c2bb7ace197528
config: i386-randconfig-s001-20210413 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-280-g2cd6d34e-dirty
        # https://github.com/0day-ci/linux/commit/dbe8fdaf74553572909be6f118565862c4cdfac5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review qianjun-kernel-gmail-com/sched-fair-Reduce-unnecessary-check-preempt-in-the-sched-tick/20210413-212057
        git checkout dbe8fdaf74553572909be6f118565862c4cdfac5
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   kernel/sched/fair.c:871:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sched_entity *se @@     got struct sched_entity [noderef] __rcu * @@
   kernel/sched/fair.c:871:34: sparse:     expected struct sched_entity *se
   kernel/sched/fair.c:871:34: sparse:     got struct sched_entity [noderef] __rcu *
>> kernel/sched/fair.c:4386:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *tsk @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:4386:37: sparse:     expected struct task_struct *tsk
   kernel/sched/fair.c:4386:37: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:7000:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct task_struct *curr @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:7000:38: sparse:     expected struct task_struct *curr
   kernel/sched/fair.c:7000:38: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:7251:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct task_struct *curr @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:7251:38: sparse:     expected struct task_struct *curr
   kernel/sched/fair.c:7251:38: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:5383:35: sparse: sparse: marked inline, but without a definition
   kernel/sched/fair.c: note: in included file:
   kernel/sched/sched.h:1887:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:1887:9: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:1887:9: sparse:    struct task_struct *
   kernel/sched/sched.h:1732:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:1732:25: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:1732:25: sparse:    struct task_struct *
   kernel/sched/sched.h:1732:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:1732:25: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:1732:25: sparse:    struct task_struct *

vim +4386 kernel/sched/fair.c

  4373	
  4374	/*
  4375	 * Preempt the current task with a newly woken task if needed:
  4376	 */
  4377	static void
  4378	check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
  4379	{
  4380		unsigned long ideal_runtime, delta_exec;
  4381		struct sched_entity *se;
  4382		struct rq *rq = rq_of(cfs_rq);
  4383		s64 delta;
  4384	
  4385		/* If the TIF_NEED_RESCHED has been set, it is no need to check again */
> 4386		if (test_tsk_need_resched(rq->curr))
  4387			return;
  4388	
  4389		ideal_runtime = sched_slice(cfs_rq, curr);
  4390		delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
  4391		if (delta_exec > ideal_runtime) {
  4392			resched_curr(rq_of(cfs_rq));
  4393			/*
  4394			 * The current task ran long enough, ensure it doesn't get
  4395			 * re-elected due to buddy favours.
  4396			 */
  4397			clear_buddies(cfs_rq, curr);
  4398			return;
  4399		}
  4400	
  4401		/*
  4402		 * Ensure that a task that missed wakeup preemption by a
  4403		 * narrow margin doesn't have to wait for a full slice.
  4404		 * This also mitigates buddy induced latencies under load.
  4405		 */
  4406		if (delta_exec < sysctl_sched_min_granularity)
  4407			return;
  4408	
  4409		se = __pick_first_entity(cfs_rq);
  4410		delta = curr->vruntime - se->vruntime;
  4411	
  4412		if (delta < 0)
  4413			return;
  4414	
  4415		if (delta > ideal_runtime)
  4416			resched_curr(rq_of(cfs_rq));
  4417	}
  4418	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32443 bytes --]

Re: [PATCH 1/1] sched/fair:Reduce unnecessary check preempt in the sched tick

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5553 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on v5.12-rc7 next-20210413]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/qianjun-kernel-gmail-com/sched-fair-Reduce-unnecessary-check-preempt-in-the-sched-tick/20210413-212057
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 4aed8aa41524a1fc6439171881c2bb7ace197528
config: i386-randconfig-s001-20210413 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-280-g2cd6d34e-dirty
        # https://github.com/0day-ci/linux/commit/dbe8fdaf74553572909be6f118565862c4cdfac5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review qianjun-kernel-gmail-com/sched-fair-Reduce-unnecessary-check-preempt-in-the-sched-tick/20210413-212057
        git checkout dbe8fdaf74553572909be6f118565862c4cdfac5
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   kernel/sched/fair.c:871:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sched_entity *se @@     got struct sched_entity [noderef] __rcu * @@
   kernel/sched/fair.c:871:34: sparse:     expected struct sched_entity *se
   kernel/sched/fair.c:871:34: sparse:     got struct sched_entity [noderef] __rcu *
>> kernel/sched/fair.c:4386:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *tsk @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:4386:37: sparse:     expected struct task_struct *tsk
   kernel/sched/fair.c:4386:37: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:7000:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct task_struct *curr @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:7000:38: sparse:     expected struct task_struct *curr
   kernel/sched/fair.c:7000:38: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:7251:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct task_struct *curr @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:7251:38: sparse:     expected struct task_struct *curr
   kernel/sched/fair.c:7251:38: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:5383:35: sparse: sparse: marked inline, but without a definition
   kernel/sched/fair.c: note: in included file:
   kernel/sched/sched.h:1887:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:1887:9: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:1887:9: sparse:    struct task_struct *
   kernel/sched/sched.h:1732:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:1732:25: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:1732:25: sparse:    struct task_struct *
   kernel/sched/sched.h:1732:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:1732:25: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:1732:25: sparse:    struct task_struct *

vim +4386 kernel/sched/fair.c

  4373	
  4374	/*
  4375	 * Preempt the current task with a newly woken task if needed:
  4376	 */
  4377	static void
  4378	check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
  4379	{
  4380		unsigned long ideal_runtime, delta_exec;
  4381		struct sched_entity *se;
  4382		struct rq *rq = rq_of(cfs_rq);
  4383		s64 delta;
  4384	
  4385		/* If the TIF_NEED_RESCHED has been set, it is no need to check again */
> 4386		if (test_tsk_need_resched(rq->curr))
  4387			return;
  4388	
  4389		ideal_runtime = sched_slice(cfs_rq, curr);
  4390		delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
  4391		if (delta_exec > ideal_runtime) {
  4392			resched_curr(rq_of(cfs_rq));
  4393			/*
  4394			 * The current task ran long enough, ensure it doesn't get
  4395			 * re-elected due to buddy favours.
  4396			 */
  4397			clear_buddies(cfs_rq, curr);
  4398			return;
  4399		}
  4400	
  4401		/*
  4402		 * Ensure that a task that missed wakeup preemption by a
  4403		 * narrow margin doesn't have to wait for a full slice.
  4404		 * This also mitigates buddy induced latencies under load.
  4405		 */
  4406		if (delta_exec < sysctl_sched_min_granularity)
  4407			return;
  4408	
  4409		se = __pick_first_entity(cfs_rq);
  4410		delta = curr->vruntime - se->vruntime;
  4411	
  4412		if (delta < 0)
  4413			return;
  4414	
  4415		if (delta > ideal_runtime)
  4416			resched_curr(rq_of(cfs_rq));
  4417	}
  4418	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32443 bytes --]

Re: [PATCH 1/1] sched/fair:Reduce unnecessary check preempt in the sched tick

[jun qian]

Peter Zijlstra <peterz@infradead.org> 于2021年4月13日周二 下午10:24写道：
>
> On Tue, Apr 13, 2021 at 09:18:42PM +0800, qianjun.kernel@gmail.com wrote:
> > From: jun qian <qianjun.kernel@gmail.com>
> >
> > If it has been determined that the current cpu need resched in the
> > early stage of for_each_sched_entity, then there is no need to check
> > preempt in the subsequent se->parent entity_tick.
>
> Right, but does it actually do anything, except increase linecount?
>
> > Signed-off-by: jun qian <qianjun.kernel@gmail.com>
> > ---
> >  kernel/sched/fair.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 1a68a0536add..c0d135100d54 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -4352,8 +4352,13 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> >  {
> >       unsigned long ideal_runtime, delta_exec;
> >       struct sched_entity *se;
> > +     struct rq *rq = rq_of(cfs_rq);
> >       s64 delta;
> >
> > +     /* If the TIF_NEED_RESCHED has been set, it is no need to check again */
> > +     if (test_tsk_need_resched(rq->curr))
> > +             return;
> > +
> >       ideal_runtime = sched_slice(cfs_rq, curr);
> >       delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
> >       if (delta_exec > ideal_runtime) {
>
> Also, I think that's placed wrong; this way we can mis clear_buddies().

thanks, i will modify it in the next version