sched: fair-group: fix a Div0 error of the fair group scheduler
PATCH 1/1 sched: fair-group: fix a Div0 error of the fair group scheduler
When I echoed 0 into the "cpu.shares" file, a Div0 error occured.
We found it is caused by the following calling.
sched_group_set_shares(tg, shares)
set_se_shares(tg->se[i], shares/nr_cpu_ids)
__set_se_shares(se, shares)
div64_64((1ULL<<32), shares)When the echoed value was less than the number of processores, the result of the sentence "shares/nr_cpu_ids" was 0, and then the system called div64() to divide the result, the Div0 error occured.
It is unnecessary that the shares value is divided by nr_cpu_ids, I think. Because in the function __update_group_shares_cpu() and init_tg_cfs_entry(), the shares value isn't divided by nr_cpu_ids when setting shares of the sched entity.
This patch fixes this bug. And echoing ULONG_MAX value into cpu.shares also causes Div0 error, so we set a macro MAX_SHARES to limit the max value of shares.
--- kernel/sched.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)
kernel/sched.c +11 -6
@@ -318,7 +318,13 @@ static DEFINE_MUTEX(doms_cur_mutex);# define INIT_TASK_GROUP_LOAD NICE_0_LOAD#endif +/* +* A weight of 0, 1 or ULONG_MAX can cause arithmetics problems. +* (The default weight is 1024 - so there's no practical +* limitation from this.) +*/ #define MIN_SHARES 2 +#define MAX_SHARES (ULONG_MAX - 1)static int init_task_group_load =INIT_TASK_GROUP_LOAD ;#endif
@@ -1748,6 +1754,8 @@ __update_group_shares_cpu(struct task_group *tg, struct sched_domain *sd,if (shares <MIN_SHARES ) shares =MIN_SHARES ; +else if (shares >MAX_SHARES ) + shares =MAX_SHARES ;__set_se_shares (tg->se[tcpu], shares); }
@@ -8722,13 +8730,10 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)if (!tg->se[0 ])return -EINVAL ; -/* -* A weight of 0 or 1 can cause arithmetics problems. -* (The default weight is 1024 - so there's no practical -* limitation from this.) -*/ if (shares <MIN_SHARES ) shares =MIN_SHARES ; +else if (shares >MAX_SHARES ) + shares =MAX_SHARES ;mutex_lock (&shares_mutex);if (tg->shares == shares)
@@ -8753,7 +8758,7 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)* force a rebalance */ cfs_rq_set_shares (tg->cfs_rq[i],0 ); -set_se_shares (tg->se[i], shares/nr_cpu_ids); +set_se_shares (tg->se[i], shares); }/*
Signature · 1 line
1.5.4.rc3
Quoted 13 lines · Miao Xie
When I echoed 0 into the "cpu.shares" file, a Div0 error occured.
We found it is caused by the following calling.
sched_group_set_shares(tg, shares) set_se_shares(tg->se[i], shares/nr_cpu_ids) __set_se_shares(se, shares) div64_64((1ULL<<32), shares)When the echoed value was less than the number of processores, the result of the sentence "shares/nr_cpu_ids" was 0, and then the system called
div64()to divide the result, the Div0 error occured.
Quoted 8 lines
It is unnecessary that the shares value is divided by
nr_cpu_ids, I think. Because in the function__update_group_shares_cpu()andinit_tg_cfs_entry(), the shares value isn't divided bynr_cpu_idswhen setting shares of the sched entity.This patch fixes this bug. And echoing
ULONG_MAXvalue into cpu.shares also causes Div0 error, so we set a macroMAX_SHARESto limit the max value of shares.
how about:
kernel/sched.c +2 -2
@@ -8025,7 +8025,7 @@ static void init_tg_cfs_entry(struct task_group *tg, struct cfs_rq *cfs_rq, se->my_q = cfs_rq; se->load.weight = tg->shares; - se->load.inv_weight =div64_64 (1ULL <<32 , se->load.weight); + se->load.inv_weight =0 ; se->parent = parent; }#endif
@@ -8692,7 +8692,7 @@ static void __set_se_shares(struct sched_entity *se, unsigned long shares)dequeue_entity (cfs_rq, se,0 ); se->load.weight = shares; - se->load.inv_weight =div64_64 ((1ULL <<32 ), shares); + se->load.inv_weight =0 ;if (on_rq)enqueue_entity (cfs_rq, se,0 );
Quoted 40 lines · on 2008-4-28 13:45 Peter Zijlstra wrote:
Quoted 13 lines · Miao Xie
When I echoed 0 into the "cpu.shares" file, a Div0 error occured.
We found it is caused by the following calling.
sched_group_set_shares(tg, shares) set_se_shares(tg->se[i], shares/nr_cpu_ids) __set_se_shares(se, shares) div64_64((1ULL<<32), shares)When the echoed value was less than the number of processores, the result of the sentence "shares/nr_cpu_ids" was 0, and then the system called
div64()to divide the result, the Div0 error occured.Quoted 8 lines
It is unnecessary that the shares value is divided by
nr_cpu_ids, I think. Because in the function__update_group_shares_cpu()andinit_tg_cfs_entry(), the shares value isn't divided bynr_cpu_idswhen setting shares of the sched entity.This patch fixes this bug. And echoing
ULONG_MAXvalue into cpu.shares also causes Div0 error, so we set a macroMAX_SHARESto limit the max value of shares.how about:
kernel/sched.c +1
-1@@ -8025,7 +8025,7 @@ static void init_tg_cfs_entry(struct task_group *tg, struct cfs_rq *cfs_rq, se->my_q = cfs_rq; se->load.weight = tg->shares; - se->load.inv_weight =div64_64 (1ULL <<32 , se->load.weight); + se->load.inv_weight =0 ; se->parent = parent; }#endif
kernel/sched.c static void __set_se_shares(struct sched_entity *se, unsigned long shares) · line 8698
Quoted 8 lines
kernel/sched.c +1
-1@@ -8692,7 +8692,7 @@ static void __set_se_shares(struct sched_entity *se, unsigned long shares)dequeue_entity (cfs_rq, se,0 ); se->load.weight = shares; - se->load.inv_weight =div64_64 ((1ULL <<32 ), shares); + se->load.inv_weight =0 ;if (on_rq)enqueue_entity (cfs_rq, se,0 );
I'm sorry, I didn't explained clearly.Though echoing ULONG_MAX value into cpu.shares causes Div0 error, this error was not caused by the above code. It is caused by the following code.
calc_delta_mine()
->lw->inv_weight = (WMULT_CONST-lw->weight/2) / (lw->weight+1);And besides, the Div0 error caused by echoing ULONG_MAX occured on the UP system or on SMP system with only one cpu.
So this patch fixes the bug caused by echoing a number which was less than the number of processores into the "cpu.shares" file, but doesn't fix the bug caused by echoing ULONG_MAX.
Quoted 7 lines · Miao Xie
I'm sorry, I didn't explained clearly.Though echoing
ULONG_MAXvalue into cpu.shares causes Div0 error, this error was not caused by the above code. It is caused by the following code.calc_delta_mine() ->lw->inv_weight = (WMULT_CONST-lw->weight/2) / (lw->weight+1);
Quoted 6 lines
And besides, the Div0 error caused by echoing
ULONG_MAXoccured on the UP system or on SMP system with only one cpu.So this patch fixes the bug caused by echoing a number which was less than the number of processores into the "cpu.shares" file, but doesn't fix the bug caused by echoing
ULONG_MAX.
Ah, ok. But I guess we run into other problems with weights that heavy, but we should not crash.
Ok, I'll ACK your initial patch
Quoted 18 lines · Miao Xie
When I echoed 0 into the "cpu.shares" file, a Div0 error occured.
We found it is caused by the following calling.
sched_group_set_shares(tg, shares) set_se_shares(tg->se[i], shares/nr_cpu_ids) __set_se_shares(se, shares) div64_64((1ULL<<32), shares)When the echoed value was less than the number of processores, the result of the sentence "shares/nr_cpu_ids" was 0, and then the system called
div64()to divide the result, the Div0 error occured.It is unnecessary that the shares value is divided by
nr_cpu_ids, I think. Because in the function__update_group_shares_cpu()andinit_tg_cfs_entry(), the shares value isn't divided bynr_cpu_idswhen setting shares of the sched entity.
Quoted 5 lines
This patch fixes this bug. And echoing
ULONG_MAXvalue into cpu.shares also causes Div0 error, so we set a macroMAX_SHARESto limit the max value of shares.Signed-off-by Miao Xie <[email protected]>
Quoted 56 lines
--- kernel/sched.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)kernel/sched.c +11
-6@@ -318,7 +318,13 @@ static DEFINE_MUTEX(doms_cur_mutex);# define INIT_TASK_GROUP_LOAD NICE_0_LOAD#endif +/* +* A weight of 0, 1 or ULONG_MAX can cause arithmetics problems. +* (The default weight is 1024 - so there's no practical +* limitation from this.) +*/ #define MIN_SHARES 2 +#define MAX_SHARES (ULONG_MAX - 1)static int init_task_group_load =INIT_TASK_GROUP_LOAD ;#endif @@ -1748,6 +1754,8 @@ __update_group_shares_cpu(struct task_group *tg, struct sched_domain *sd,if (shares <MIN_SHARES ) shares =MIN_SHARES ; +else if (shares >MAX_SHARES ) + shares =MAX_SHARES ;__set_se_shares (tg->se[tcpu], shares); }@@ -8722,13 +8730,10 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)if (!tg->se[0 ])return -EINVAL ; -/* -* A weight of 0 or 1 can cause arithmetics problems. -* (The default weight is 1024 - so there's no practical -* limitation from this.) -*/ if (shares <MIN_SHARES ) shares =MIN_SHARES ; +else if (shares >MAX_SHARES ) + shares =MAX_SHARES ;mutex_lock (&shares_mutex);if (tg->shares == shares)@@ -8753,7 +8758,7 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)* force a rebalance */ cfs_rq_set_shares (tg->cfs_rq[i],0 ); -set_se_shares (tg->se[i], shares/nr_cpu_ids); +set_se_shares (tg->se[i], shares); }/*
Ingo Molnar
AppliedQuoted 13 lines · Miao Xie
When I echoed 0 into the "cpu.shares" file, a Div0 error occured.
We found it is caused by the following calling.
sched_group_set_shares(tg, shares)set_se_shares(tg->se[i], shares/nr_cpu_ids) __set_se_shares(se, shares) div64_64((1ULL<<32), shares)When the echoed value was less than the number of processores, the result of the sentence "shares/nr_cpu_ids" was 0, and then the system called
div64()to divide the result, the Div0 error occured.
Quoted 8 lines
It is unnecessary that the shares value is divided by
nr_cpu_ids, I think. Because in the function__update_group_shares_cpu()andinit_tg_cfs_entry(), the shares value isn't divided bynr_cpu_idswhen setting shares of the sched entity.This patch fixes this bug. And echoing
ULONG_MAXvalue into cpu.shares also causes Div0 error, so we set a macroMAX_SHARESto limit the max value of shares.
thanks, applied.
Ingo