sched: Avoid unnecessary work in reweight_entity

From: Michael Wang <[email protected]>

In original code, using account_entity_dequeue and account_entity_enqueue as a pair will do some work unnecessary, this patch combine the work of them to save time.

Signed-off-by Michael Wang <[email protected]>

---
 kernel/sched/fair.c  |   32 ++++++++++++++++++++++++--------
 kernel/sched/sched.h |    8 ++++++++
 2 files changed, 32 insertions(+), 8 deletions(-)

kernel/sched/fair.c +12 -0

index 84adb2d..e380518 100644
@@ -782,11 +782,23 @@ add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long weight)
 {
 	cfs_rq->task_weight += weight;
 }
+
+static 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;
+}
 #else
 static inline void
 add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long weight)
 {
 }
+
+static void
+sub_add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long sub, unsigned long add)
+{
+}
 #endif
 
 static void

kernel/sched/fair.c +1 -4

+
 
 	if (se->on_rq) {
-		/* commit outstanding execution time */
-		if (cfs_rq->curr == se)
-			update_curr(cfs_rq);
-		account_entity_dequeue(cfs_rq, se);

kernel/sched/fair.c +0 -3

-
-	if (se->on_rq)
-		account_entity_enqueue(cfs_rq, se);
 }
 

static void update_cfs_shares(struct cfs_rq *cfs_rq)

kernel/sched/sched.h +8 -0

index 98c0c26..ec4430f 100644
@@ -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;
+}
+
 static inline void update_load_set(struct load_weight *lw, unsigned long w)
 {
 	lw->weight = w;

From: Michael Wang <[email protected]>

In original code, using account_entity_dequeue and account_entity_enqueue as a pair will do some work unnecessary, this patch combine the work of them to save time.

Signed-off-by Michael Wang <[email protected]>

---
 kernel/sched/fair.c  |   32 ++++++++++++++++++++++++--------
 kernel/sched/sched.h |    8 ++++++++
 2 files changed, 32 insertions(+), 8 deletions(-)

kernel/sched/fair.c +15 -0

index 84adb2d..e380518 100644
@@ -782,11 +782,23 @@ add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long weight)
 {
 	cfs_rq->task_weight += weight;
 }
+
+static 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;
+}
 #else
 static inline void
 add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long weight)
 {
 }
+
+static void
+sub_add_cfs_task_weight(struct cfs_rq *cfs_rq, unsigned long sub, unsigned long add)
+{
+}
 #endif
 
 static void

@@ -938,20 +950,24 @@ 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)
 {
+	/* commit outstanding execution time */
+	if (cfs_rq->curr == se)

kernel/sched/fair.c +4 -4

+		}
+		if (entity_is_task(se)) {
+			sub_add_cfs_task_weight(cfs_rq, se->load.weight, weight);
+		}
 	}
-
 	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)

kernel/sched/sched.h +8 -0

index 98c0c26..ec4430f 100644
@@ -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;
+}
+
 static inline void update_load_set(struct load_weight *lw, unsigned long w)
 {
 	lw->weight = w;

From: Michael Wang <[email protected]>

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.

Signed-off-by Michael Wang <[email protected]>

---
 kernel/sched/fair.c  |   27 +++++++++++++++++++++------
 kernel/sched/sched.h |    8 ++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

kernel/sched/fair.c +1 -1

index 7c6414f..fda63ec 100644
@@ -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)

kernel/sched/fair.c +6 -0

 #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)
 {

kernel/sched/fair.c +0 -4

 	}
-
 	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)

kernel/sched/sched.h +0 -0

index 98c0c26..ec4430f 100644

kernel/sched/sched.h +0 -0

 static inline void update_load_set(struct load_weight *lw, unsigned long w)
 {
 	lw->weight = w;

Quoted 6 lines · Michael Wang

From: Michael Wang <[email protected]>

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.

Quoted 4 lines

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 :-(

Quoted 13 lines

Signed-off-by Michael Wang <[email protected]>

---
 kernel/sched/fair.c  |   27 +++++++++++++++++++++------
 kernel/sched/sched.h |    8 ++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

kernel/sched/fair.c +0 -0

index 7c6414f..fda63ec 100644
@@ -777,16 +777,28 @@ update_stats_curr_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
  */
 

Quoted 4 lines

kernel/sched/fair.c +1 -1

 #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)

kernel/sched/fair.c update_stats_curr_start(struct cfs_rq *cfs_rq, struct sched_entity *se) · line 791

Quoted 10 lines

kernel/sched/fair.c +7 -0

 {
 	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);

Quoted 22 lines

kernel/sched/fair.c +6 -0

 #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)
 {

kernel/sched/fair.c static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, · line 963

Quoted 7 lines

kernel/sched/fair.c +3 -1

@@ -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.

kernel/sched/fair.c static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, · line 967

Quoted 4 lines

kernel/sched/fair.c +4 -0

+		}
+		if (entity_is_task(se)) {
+			sub_add_cfs_task_weight(cfs_rq, se->load.weight, weight);
+		}

idem.

Quoted 14 lines

kernel/sched/fair.c +0 -4

 	}
-
 	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)

kernel/sched/sched.h +0 -0

index 98c0c26..ec4430f 100644

kernel/sched/sched.h static inline void update_load_sub(struct load_weight *lw, unsigned long dec) · line 788

Quoted 10 lines

kernel/sched/sched.h +7 -0

@@ -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.

Quoted 3 lines

kernel/sched/sched.h +0 -0

 static inline void update_load_set(struct load_weight *lw, unsigned long w)
 {
 	lw->weight = w;

From: Michael Wang <[email protected]>

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.

since v1:
Add more description.
Mark add_cfs_task_weight as inline.
correct useless and wrong code.
Add check point for the case there is no change on weight.

Signed-off-by Michael Wang <[email protected]>

---
 kernel/sched/fair.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

kernel/sched/fair.c +10 -6

index 7c6414f..20aa355 100644
@@ -777,7 +777,7 @@ 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;

@@ -938,20 +938,24 @@ 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)
 {
+	if (weight == se->load.weight)
+		return;
+
 	if (se->on_rq) {
 		/* commit outstanding execution time */
 		if (cfs_rq->curr == se)
 			update_curr(cfs_rq);
-		account_entity_dequeue(cfs_rq, se);
+		update_load_add(&cfs_rq->load, weight - se->load.weight);
+		if (!parent_entity(se))
+			update_load_add(&rq_of(cfs_rq)->load, weight - se->load.weight);
+		if (entity_is_task(se))
+			add_cfs_task_weight(cfs_rq, weight - se->load.weight);
 	}
-
 	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)

And as reight_entity is invoked very often, I think this patch can do some help to the performance, although there are no numbers, we can prove it logically :)

Quoted 2 lines

And as reight_entity is invoked very often, I think this patch can do some help to the performance, although there are no numbers, we can prove it logically :)

Well, you're probably right (although I think you completely ignored the optimizing compiler), but still it would be good to get some numbers to confirm reality :-)

Just pick your favourite cgroup workload/benchmark and run a pre/post comparison, possible using perf record.

If all squares up you should see an improvement in your benchmark score as well as see a reduction in time spend in the function you're patching.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On 02/20/2012 09:08 PM, Peter Zijlstra wrote:

Hi, Peter

Sorry for reply so late, I was blocked by some issue army while setup the testing environment.

Quoted 7 lines · Michael Wang

Quoted 2 lines

And as reight_entity is invoked very often, I think this patch can do some help to the performance, although there are no numbers, we can prove it logically :)

Well, you're probably right (although I think you completely ignored the optimizing compiler), but still it would be good to get some numbers to confirm reality :-)

That's right, if consider the compiler's optimization, the logic improvements I listed may not be true...

Quoted 8 lines

Just pick your favourite cgroup workload/benchmark and run a pre/post comparison, possible using perf record.

If all squares up you should see an improvement in your benchmark score as well as see a reduction in time spend in the function you're patching.

So I created a cpuset cgroup 'rg1' and his sub memory group 'sub', attached current shell to 'sub', then use 'time make kernel' as benchmark.

Below is the test result:

'time make':
old

	real: 87m53.804s	user: 66m41.886s	sys: 11m51.792s
new
	real: 86m8.485s		user: 65m49.211s	sys: 11m47.264s

'time make -j14':
old:

	real: 42m43.825s	user: 124m13.726s	sys: 17m57.183s
new
	real: 39m0.072s		user: 114m33.258s	sys: 15m30.662s

I also try to use 'perf sched record', but I can only record few seconds time, otherwise it will be too big and report some error, as the record time is too short, results are very different from each other, I failed to use them to prove the patch:(

I also have try to use some other method, I moved 'reweight_entity' and related functions to user space, and invoke it 10000000 times in 'main', I have append part of the code (really raw...) in the end.

Three times output is:

old:

	real	0m0.715s	0m0.710s	0m0.739s
	user	0m0.716s	0m0.708s	0m0.736s
	sys	0m0.000s	0m0.000s	0m0.000s

new:

	real	0m0.318s	0m0.308s	0m0.317s
	user	0m0.316s	0m0.304s	0m0.316s
	sys	0m0.000s	0m0.000s	0m0.000s

It seems like that new code can save more then half execution time, but also we can see, after calculation, what I have done can only save 0.04ns(too small...).

The user space test result is not accurate but at least we can know new code is faster then old.

Please tell me if we need to do some thing else, and thanks for your suggestion :)

Regards,
Michael Wang

User space code:

void
account_entity_enqueue(struct cfs_rq *cfs_rq, struct sched_entity *se)

{
        update_load_add(&cfs_rq->load, se->load.weight);
        if (1)
                update_load_add(&cfs_rq->load, se->load.weight);
        if (1) {
                add_cfs_task_weight(cfs_rq, se->load.weight);
                list_add(&se->group_node, &cfs_rq->tasks);
        }
        cfs_rq->nr_running++;
}

void
account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)

{
        update_load_sub(&cfs_rq->load, se->load.weight);
        if (1)
                update_load_sub(&cfs_rq->load, se->load.weight);
        if (1) {
                add_cfs_task_weight(cfs_rq, -se->load.weight);
                list_del_init(&se->group_node);
        }
        cfs_rq->nr_running--;
}

void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
unsigned long weight)

{
        if (1) {
                account_entity_dequeue(cfs_rq, se);
        }

        update_load_set(&se->load, weight);

        if (1)
                account_entity_enqueue(cfs_rq, se);
}

void reweight_entity_new(struct cfs_rq *cfs_rq, struct sched_entity *se,
unsigned long weight)

{
        if (1) {
                update_load_add(&cfs_rq->load, weight - se->load.weight);
                if(1)
                        update_load_add(&cfs_rq->load, weight -
se->load.weight);
                if(1)
                        add_cfs_task_weight(cfs_rq, weight
-se->load.weight);
        }
        update_load_set(&se->load, weight);
}

int main()

{
        struct cfs_rq cfsrq;
        struct sched_entity se;
        memset(&cfsrq, 0, sizeof(struct cfs_rq));
        memset(&se, 0, sizeof(struct sched_entity));
        INIT_LIST_HEAD(&se.group_node);
        INIT_LIST_HEAD(&cfsrq.tasks);
        int i = 10000000;
        while(i) {
                i--;
                reweight_entity_new(&cfsrq, &se, 10);
                //reweight_entity(&cfsrq, &se, 10);
        }
}

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 40 lines · Michael Wang

On 02/20/2012 09:08 PM, Peter Zijlstra wrote:

Hi, Peter

Sorry for reply so late, I was blocked by some issue army while setup the testing environment.

Quoted 7 lines · Michael Wang

Quoted 2 lines

And as reight_entity is invoked very often, I think this patch can do some help to the performance, although there are no numbers, we can prove it logically :)

Well, you're probably right (although I think you completely ignored the optimizing compiler), but still it would be good to get some numbers to confirm reality :-)

That's right, if consider the compiler's optimization, the logic improvements I listed may not be true...

Quoted 8 lines

Just pick your favourite cgroup workload/benchmark and run a pre/post comparison, possible using perf record.

If all squares up you should see an improvement in your benchmark score as well as see a reduction in time spend in the function you're patching.

So I created a cpuset cgroup 'rg1' and his sub memory group 'sub', attached current shell to 'sub', then use 'time make kernel' as benchmark.

Below is the test result:

'time make':
old

	real: 87m53.804s	user: 66m41.886s	sys: 11m51.792s
new
	real: 86m8.485s		user: 65m49.211s	sys: 11m47.264s

Quoted 6 lines

'time make -j14':
old:

	real: 42m43.825s	user: 124m13.726s	sys: 17m57.183s
new
	real: 39m0.072s		user: 114m33.258s	sys: 15m30.662s

Hi, Peter

Someone notify me that this result is ridiculous, I should have done more test, not just once, this is really my fault, please give me more time, I will back with more data so we can use average number.

Regards,
Michael Wang

Quoted 120 lines

I also try to use 'perf sched record', but I can only record few seconds time, otherwise it will be too big and report some error, as the record time is too short, results are very different from each other, I failed to use them to prove the patch:(

I also have try to use some other method, I moved 'reweight_entity' and related functions to user space, and invoke it 10000000 times in 'main', I have append part of the code (really raw...) in the end.

Three times output is:

old:

	real	0m0.715s	0m0.710s	0m0.739s
	user	0m0.716s	0m0.708s	0m0.736s
	sys	0m0.000s	0m0.000s	0m0.000s

new:

	real	0m0.318s	0m0.308s	0m0.317s
	user	0m0.316s	0m0.304s	0m0.316s
	sys	0m0.000s	0m0.000s	0m0.000s

It seems like that new code can save more then half execution time, but also we can see, after calculation, what I have done can only save 0.04ns(too small...).

The user space test result is not accurate but at least we can know new code is faster then old.

Please tell me if we need to do some thing else, and thanks for your suggestion :)

Regards,
Michael Wang

User space code:

void
account_entity_enqueue(struct cfs_rq *cfs_rq, struct sched_entity *se)

{
        update_load_add(&cfs_rq->load, se->load.weight);
        if (1)
                update_load_add(&cfs_rq->load, se->load.weight);
        if (1) {
                add_cfs_task_weight(cfs_rq, se->load.weight);
                list_add(&se->group_node, &cfs_rq->tasks);
        }
        cfs_rq->nr_running++;
}

void
account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)

{
        update_load_sub(&cfs_rq->load, se->load.weight);
        if (1)
                update_load_sub(&cfs_rq->load, se->load.weight);
        if (1) {
                add_cfs_task_weight(cfs_rq, -se->load.weight);
                list_del_init(&se->group_node);
        }
        cfs_rq->nr_running--;
}

void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
unsigned long weight)

{
        if (1) {
                account_entity_dequeue(cfs_rq, se);
        }

        update_load_set(&se->load, weight);

        if (1)
                account_entity_enqueue(cfs_rq, se);
}

void reweight_entity_new(struct cfs_rq *cfs_rq, struct sched_entity *se,
unsigned long weight)

{
        if (1) {
                update_load_add(&cfs_rq->load, weight - se->load.weight);
                if(1)
                        update_load_add(&cfs_rq->load, weight -
se->load.weight);
                if(1)
                        add_cfs_task_weight(cfs_rq, weight
-se->load.weight);
        }
        update_load_set(&se->load, weight);
}

int main()

{
        struct cfs_rq cfsrq;
        struct sched_entity se;
        memset(&cfsrq, 0, sizeof(struct cfs_rq));
        memset(&se, 0, sizeof(struct sched_entity));
        INIT_LIST_HEAD(&se.group_node);
        INIT_LIST_HEAD(&cfsrq.tasks);
        int i = 10000000;
        while(i) {
                i--;
                reweight_entity_new(&cfsrq, &se, 10);
                //reweight_entity(&cfsrq, &se, 10);
        }
}

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hi, Peter

I have collected more testing data, here is the test results:

Machine: ThinkPad T420
OS: Ubuntu 11.10
Benchmark: time make -j14 (build kernel)

Quoted 7 lines · Michael Wang

Hi, Peter

I have collected more testing data, here is the test results:

Machine: ThinkPad T420
OS: Ubuntu 11.10
Benchmark: time make -j14 (build kernel)

Is that benchmark run in root (cpu) cgroup? If so, reweight_entity() should not kick in at all.

static void update_cfs_shares(struct cfs_rq *cfs_rq) {

        ..

        if (!se || throttled_hierarchy(cfs_rq))
                return;

        ..

	reweight_entity();
}

If you want to stress reweight_entity() create several (cpu) cgroups and launch workload like kernbench in each of them ..

- vatsa

Quoted 2 lines

Is that benchmark run in root (cpu) cgroup? If so, reweight_entity() should not kick in at all.

That's right, if no children group, 'reweight_entity' won't be called, so I have created a cpuset group under root group named 'rg1', and created a memory group under 'rg1' named 'sub', I attached the current shell to the 'sub' cgroup.

Quoted 4 lines · Michael Wang

Quoted 2 lines

Is that benchmark run in root (cpu) cgroup? If so, reweight_entity() should not kick in at all.

Quoted 3 lines

That's right, if no children group, 'reweight_entity' won't be called, so I have created a cpuset group under root group named 'rg1', and created a memory group under 'rg1' named 'sub', I attached the current shell to the 'sub' cgroup.

'cpu' and 'cpuset' cgroup resource controllers are separate. By above steps, you would still be running kern-bench in root cgroup as far as cpu cgroup controller is concerned ..

- vatsa

I think I really need some study on cgroup first...I've done some totally useless test :( ,

but still confused that why sys time reduced?

And I got a server with Redhat now, so I will do:

1. cgcreate -g cpu:/subcg1
2. echo $$ > /cgroup/cpu/subcg1/tasks
3. open another shell
4. do 1~3 multi times, plan to get 7 cpu cgroup with 7 shell attached to each one.
5. run kernbench in each shell

Please tell me if I missed some thing :)

From: Michael Wang <[email protected]>

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.

v2:
Add more description and revise wrong code.

v3:
Mark add_cfs_task_weight and sub_add_cfs_task_weight as inline.

v4:
Now we invoke update_curr when really necessary.

Signed-off-by Michael Wang <[email protected]>

---
 kernel/sched/fair.c  |   27 +++++++++++++++++++++------
 kernel/sched/sched.h |    8 ++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

kernel/sched/fair.c +21 -6

index 7c6414f..fda63ec 100644
@@ -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;
+}
 #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);
+		}
+		if (entity_is_task(se)) {
+			sub_add_cfs_task_weight(cfs_rq, se->load.weight, weight);
+		}
 	}
-
 	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)

kernel/sched/sched.h +8 -0

index 98c0c26..ec4430f 100644
@@ -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;
+}
+
 static inline void update_load_set(struct load_weight *lw, unsigned long w)
 {
 	lw->weight = w;