From: jun qian <[email protected]>
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 <[email protected]>
---
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
On Tue, Apr 13, 2021 at 09:18:42PM +0800, [email protected] wrote:
> From: jun qian <[email protected]>
>
> 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 <[email protected]>
> ---
> 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().
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 <[email protected]>
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/[email protected]
Peter Zijlstra <[email protected]> 于2021年4月13日周二 下午10:24写道：
>
> On Tue, Apr 13, 2021 at 09:18:42PM +0800, [email protected] wrote:
> > From: jun qian <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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