lkml.org 
[lkml]   [2012]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4] sched: Avoid unnecessary work in reweight_entity
    Hi, Peter

    Thanks for your reply.

    On 02/16/2012 10:29 PM, Peter Zijlstra wrote:

    > On Thu, 2012-02-16 at 22:14 +0800, Michael Wang wrote:
    >> From: Michael Wang <wangyun@linux.vnet.ibm.com>
    >>
    >> In original code, using account_entity_dequeue and account_entity_enqueue
    >> as a pair will do some work unnecessary, such like remove node from list
    >> and add it back again immediately.
    >>
    >> And because there are no really enqueue and dequeue happen here, it is not
    >> suitable to use account_entity_dequeue and account_entity_enqueue here.
    >>
    >> This patch combine the work of them in order to save time.
    >
    > And yet no numbers on how much it saves :-(
    >


    That's right...
    Actually I don't know how to prove the performance improvement, please
    give me some suggestion and I will do it :)

    >> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
    >> ---
    >> kernel/sched/fair.c | 27 +++++++++++++++++++++------
    >> kernel/sched/sched.h | 8 ++++++++
    >> 2 files changed, 29 insertions(+), 6 deletions(-)
    >>
    >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
    >> index 7c6414f..fda63ec 100644
    >> --- a/kernel/sched/fair.c
    >> +++ b/kernel/sched/fair.c
    >> @@ -777,16 +777,28 @@ update_stats_curr_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
    >> */
    >>
    >> #if defined CONFIG_SMP && defined CONFIG_FAIR_GROUP_SCHED
    >> -static void
    >> +static inline void
    >> add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long weight)
    >> {
    >> cfs_rq->task_weight += weight;
    >> }
    >> +
    >> +static inline void
    >> +sub_add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long sub, unsigned long add)
    >> +{
    >> + cfs_rq->task_weight -= sub;
    >> + cfs_rq->task_weight += add;
    >> +}
    >
    > This is pointless, just use: add_cfs_task_weight(cfs_rq, add - sub);
    >


    Sorry for the silly code, v5 will correct it and all the issues mentioned below.

    Regards,
    Michael Wang

    >> #else
    >> static inline void
    >> add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long weight)
    >> {
    >> }
    >> +
    >> +static inline void
    >> +sub_add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long sub, unsigned long add)
    >> +{
    >> +}
    >> #endif
    >>
    >> static void
    >> @@ -938,6 +950,7 @@ static inline void update_entity_shares_tick(struct cfs_rq *cfs_rq)
    >> {
    >> }
    >> # endif /* CONFIG_SMP */
    >> +
    >> static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
    >> unsigned long weight)
    >> {
    >> @@ -945,13 +958,15 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
    >> /* commit outstanding execution time */
    >> if (cfs_rq->curr == se)
    >> update_curr(cfs_rq);
    >> - account_entity_dequeue(cfs_rq, se);
    >> + update_load_sub_add(&cfs_rq->load, se->load.weight, weight);
    >> + if (!parent_entity(se)) {
    >> + update_load_sub_add(&rq_of(cfs_rq)->load, se->load.weight, weight);
    >
    > same here:
    > update_load_add(&rq_of(cfs_rq)->load, weight - se->load.weight);
    >
    > Also superfluous use of curly-braces here.
    >
    >> + }
    >> + if (entity_is_task(se)) {
    >> + sub_add_cfs_task_weight(cfs_rq, se->load.weight, weight);
    >> + }
    >
    > idem.
    >
    >> }
    >> -
    >> update_load_set(&se->load, weight);
    >> -
    >> - if (se->on_rq)
    >> - account_entity_enqueue(cfs_rq, se);
    >> }
    >>
    >> static void update_cfs_shares(struct cfs_rq *cfs_rq)
    >> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
    >> index 98c0c26..ec4430f 100644
    >> --- a/kernel/sched/sched.h
    >> +++ b/kernel/sched/sched.h
    >> @@ -779,6 +779,14 @@ static inline void update_load_sub(struct load_weight *lw, unsigned long dec)
    >> lw->inv_weight = 0;
    >> }
    >>
    >> +static inline void update_load_sub_add(struct load_weight *lw, unsigned long dec,
    >> + unsigned long inc)
    >> +{
    >> + lw->weight -= dec;
    >> + lw->weight += inc;
    >> + lw->inv_weight = 0;
    >> +}
    >
    > again, pointless stuff.
    >
    >> static inline void update_load_set(struct load_weight *lw, unsigned long w)
    >> {
    >> lw->weight = w;
    >




    \
     
     \ /
      Last update: 2012-02-17 03:31    [W:0.031 / U:0.536 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site