From: Yong Zhang <yong.zhang0@gmail.com>
To: Paul Turner <pjt@google.com>
Cc: Wang Xingchao <xingchao.wang@intel.com>,
linux-kernel@vger.kernel.org, mingo@elte.hu,
peterz@infradead.org
Subject: Re: [PATCH] cfs: remove unused 'if' condition checking
Date: Fri, 16 Sep 2011 16:17:00 +0800 [thread overview]
Message-ID: <20110916081700.GC10741@zhy> (raw)
In-Reply-To: <4E72FD93.2060805@google.com>
On Fri, Sep 16, 2011 at 12:41:07AM -0700, Paul Turner wrote:
> On 09/16/11 10:35, Wang Xingchao wrote:
>
> Please include a one-line summary such as:
> sched: remove extra nr_running check in check_preempt_tick
>
> > nr_running must be more than 1, so remove the checking
> >
>
> Sure, reasonable.
>
> Please expand this comment though, e.g.
> We already test for nr_running > 1 within entity_tick so there is no
> need to recheck it within check_preempt_tick().
>
> > Signed-off-by: Wang Xingchao<xingchao.wang@intel.com>
>
> Reviewed-by: Paul Turner <pjt@google.com>
>
> > ---
> > kernel/sched_fair.c | 16 ++++++++--------
> > 1 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
> > index 1ca2cd4..fef0bfd 100644
> > --- a/kernel/sched_fair.c
> > +++ b/kernel/sched_fair.c
> > @@ -1106,6 +1106,8 @@ static void
> > check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> > {
> > unsigned long ideal_runtime, delta_exec;
> > + struct sched_entity *se;
> > + s64 delta;
> >
> > ideal_runtime = sched_slice(cfs_rq, curr);
> > delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
> > @@ -1127,16 +1129,14 @@ check_preempt_tick(struct cfs_rq *cfs_rq,
> struct sched_entity *curr)
> > if (delta_exec< sysctl_sched_min_granularity)
>
> You've mangled the white space on the less-than.
But it looks ok on my side.
And on LKML: https://lkml.org/lkml/2011/9/16/11
So maybe it's not Xingchao's problem.
Thanks,
Yong
>
> > return;
> >
> > - if (cfs_rq->nr_running> 1) {
> > - struct sched_entity *se = __pick_first_entity(cfs_rq);
> > - s64 delta = curr->vruntime - se->vruntime;
> > + se = __pick_first_entity(cfs_rq);
> > + delta = curr->vruntime - se->vruntime;
> >
> > - if (delta< 0)
> > - return;
> > + if (delta< 0)
>
> And here.
>
> > + return;
> >
> > - if (delta> ideal_runtime)
> > - resched_task(rq_of(cfs_rq)->curr);
> > - }
> > + if (delta> ideal_runtime)
>
> Here too.
>
> > + resched_task(rq_of(cfs_rq)->curr);
> > }
> >
> > static void
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Only stand for myself
next prev parent reply other threads:[~2011-09-16 8:17 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-16 17:35 [PATCH] cfs: remove unused 'if' condition checking Wang Xingchao
2011-09-16 5:54 ` Yong Zhang
2011-09-16 7:39 ` Paul Turner
2011-09-16 7:41 ` Paul Turner
2011-09-16 8:17 ` Yong Zhang [this message]
2011-09-16 8:28 ` Paul Turner
2011-09-16 20:36 ` xingchao
2011-09-16 8:38 ` Peter Zijlstra
2011-09-16 20:40 ` xingchao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110916081700.GC10741@zhy \
--to=yong.zhang0@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
--cc=xingchao.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox