* [patch] sched, cgroups: fix MIN_SHARES on 64 bit boxen @ 2011-06-04 9:29 Mike Galbraith 2011-06-04 11:24 ` Peter Zijlstra 0 siblings, 1 reply; 4+ messages in thread From: Mike Galbraith @ 2011-06-04 9:29 UTC (permalink / raw) To: LKML; +Cc: Nikhil Rao, Peter Zijlstra, Ingo Molnar c8b28116 claimed to have no user-visible effect, but allows setting cpu.shares to < MIN_SHARES, which the user then indeed sees. Signed-off-by: Mike Galbraith <efault@gmx.de> --- kernel/sched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6/kernel/sched.c =================================================================== --- linux-2.6.orig/kernel/sched.c +++ linux-2.6/kernel/sched.c @@ -292,7 +292,7 @@ static DEFINE_SPINLOCK(task_group_lock); * (The default weight is 1024 - so there's no practical * limitation from this.) */ -#define MIN_SHARES 2 +#define MIN_SHARES (scale_load(2)) #define MAX_SHARES (1UL << (18 + SCHED_LOAD_RESOLUTION)) static int root_task_group_load = ROOT_TASK_GROUP_LOAD; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] sched, cgroups: fix MIN_SHARES on 64 bit boxen 2011-06-04 9:29 [patch] sched, cgroups: fix MIN_SHARES on 64 bit boxen Mike Galbraith @ 2011-06-04 11:24 ` Peter Zijlstra 2011-06-04 13:03 ` Mike Galbraith 0 siblings, 1 reply; 4+ messages in thread From: Peter Zijlstra @ 2011-06-04 11:24 UTC (permalink / raw) To: Mike Galbraith; +Cc: LKML, Nikhil Rao, Ingo Molnar On Sat, 2011-06-04 at 11:29 +0200, Mike Galbraith wrote: > c8b28116 claimed to have no user-visible effect, but allows setting cpu.shares > to < MIN_SHARES, which the user then indeed sees. > > Signed-off-by: Mike Galbraith <efault@gmx.de> > --- > kernel/sched.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-2.6/kernel/sched.c > =================================================================== > --- linux-2.6.orig/kernel/sched.c > +++ linux-2.6/kernel/sched.c > @@ -292,7 +292,7 @@ static DEFINE_SPINLOCK(task_group_lock); > * (The default weight is 1024 - so there's no practical > * limitation from this.) > */ > -#define MIN_SHARES 2 > +#define MIN_SHARES (scale_load(2)) > #define MAX_SHARES (1UL << (18 + SCHED_LOAD_RESOLUTION)) > > static int root_task_group_load = ROOT_TASK_GROUP_LOAD; Hurm, but that destroys most of the gains from that patch, the whole point was being able to have finer graunlarities, but now calc_cfs_shares() and effective_load() are clipped the coarse granularity. So maybe explicitly change the MIN_SHARES usage in sched_group_set_shares(). That wants to become a clamp user anyway, something like: shares = clamp(shares, scale_load(MIN_SHARES), scale_load(MAX_SHARES)); That way MAX_SHARES can also loose its SCHED_LOAD_RESOLUTION factor and bring is back in line with MIN_SHARES. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] sched, cgroups: fix MIN_SHARES on 64 bit boxen 2011-06-04 11:24 ` Peter Zijlstra @ 2011-06-04 13:03 ` Mike Galbraith 2011-07-01 15:15 ` [tip:sched/urgent] sched, cgroups: Fix MIN_SHARES on 64-bit boxen tip-bot for Mike Galbraith 0 siblings, 1 reply; 4+ messages in thread From: Mike Galbraith @ 2011-06-04 13:03 UTC (permalink / raw) To: Peter Zijlstra; +Cc: LKML, Nikhil Rao, Ingo Molnar On Sat, 2011-06-04 at 13:24 +0200, Peter Zijlstra wrote: > On Sat, 2011-06-04 at 11:29 +0200, Mike Galbraith wrote: > > c8b28116 claimed to have no user-visible effect, but allows setting cpu.shares > > to < MIN_SHARES, which the user then indeed sees. > > > > Signed-off-by: Mike Galbraith <efault@gmx.de> > > --- > > kernel/sched.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Index: linux-2.6/kernel/sched.c > > =================================================================== > > --- linux-2.6.orig/kernel/sched.c > > +++ linux-2.6/kernel/sched.c > > @@ -292,7 +292,7 @@ static DEFINE_SPINLOCK(task_group_lock); > > * (The default weight is 1024 - so there's no practical > > * limitation from this.) > > */ > > -#define MIN_SHARES 2 > > +#define MIN_SHARES (scale_load(2)) > > #define MAX_SHARES (1UL << (18 + SCHED_LOAD_RESOLUTION)) > > > > static int root_task_group_load = ROOT_TASK_GROUP_LOAD; > > Hurm, but that destroys most of the gains from that patch, the whole > point was being able to have finer graunlarities, but now > calc_cfs_shares() and effective_load() are clipped the coarse > granularity. Oh well, drink one, spill one, give one away :) > So maybe explicitly change the MIN_SHARES usage in > sched_group_set_shares(). That wants to become a clamp user anyway, > something like: > > shares = clamp(shares, scale_load(MIN_SHARES), scale_load(MAX_SHARES)); > > That way MAX_SHARES can also loose its SCHED_LOAD_RESOLUTION factor and > bring is back in line with MIN_SHARES. sched, cgroups: fix sched_group_set_shares() on 64 bit boxen c8b28116 claimed to have no user-visible effect, but allows setting cpu.shares to < MIN_SHARES, which the user then sees. Signed-off-by: Mike Galbraith <efault@gmx.de> --- kernel/sched.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) Index: linux-2.6/kernel/sched.c =================================================================== --- linux-2.6.orig/kernel/sched.c +++ linux-2.6/kernel/sched.c @@ -292,8 +292,8 @@ static DEFINE_SPINLOCK(task_group_lock); * (The default weight is 1024 - so there's no practical * limitation from this.) */ -#define MIN_SHARES 2 -#define MAX_SHARES (1UL << (18 + SCHED_LOAD_RESOLUTION)) +#define MIN_SHARES (1UL << 1) +#define MAX_SHARES (1UL << 18) static int root_task_group_load = ROOT_TASK_GROUP_LOAD; #endif @@ -8439,10 +8439,7 @@ int sched_group_set_shares(struct task_g if (!tg->se[0]) return -EINVAL; - if (shares < MIN_SHARES) - shares = MIN_SHARES; - else if (shares > MAX_SHARES) - shares = MAX_SHARES; + shares = clamp(shares, scale_load(MIN_SHARES), scale_load(MAX_SHARES)); mutex_lock(&shares_mutex); if (tg->shares == shares) ^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip:sched/urgent] sched, cgroups: Fix MIN_SHARES on 64-bit boxen 2011-06-04 13:03 ` Mike Galbraith @ 2011-07-01 15:15 ` tip-bot for Mike Galbraith 0 siblings, 0 replies; 4+ messages in thread From: tip-bot for Mike Galbraith @ 2011-07-01 15:15 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, mgalbraith, hpa, mingo, ncrao, a.p.zijlstra, efault, tglx, mingo Commit-ID: cd62287e364c0d15d517c6ced4e4808b54711475 Gitweb: http://git.kernel.org/tip/cd62287e364c0d15d517c6ced4e4808b54711475 Author: Mike Galbraith <mgalbraith@suse.de> AuthorDate: Sat, 4 Jun 2011 15:03:20 +0200 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Fri, 1 Jul 2011 10:25:03 +0200 sched, cgroups: Fix MIN_SHARES on 64-bit boxen Commit c8b28116 ("sched: Increase SCHED_LOAD_SCALE resolution") intended to have no user-visible effect, but allows setting cpu.shares to < MIN_SHARES, which the user then sees. Signed-off-by: Mike Galbraith <efault@gmx.de> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Nikhil Rao <ncrao@google.com> Link: http://lkml.kernel.org/r/1307192600.8618.3.camel@marge.simson.net Signed-off-by: Ingo Molnar <mingo@elte.hu> --- kernel/sched.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/kernel/sched.c b/kernel/sched.c index 3f2e502..9769c75 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -292,8 +292,8 @@ static DEFINE_SPINLOCK(task_group_lock); * (The default weight is 1024 - so there's no practical * limitation from this.) */ -#define MIN_SHARES 2 -#define MAX_SHARES (1UL << (18 + SCHED_LOAD_RESOLUTION)) +#define MIN_SHARES (1UL << 1) +#define MAX_SHARES (1UL << 18) static int root_task_group_load = ROOT_TASK_GROUP_LOAD; #endif @@ -8450,10 +8450,7 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares) if (!tg->se[0]) return -EINVAL; - if (shares < MIN_SHARES) - shares = MIN_SHARES; - else if (shares > MAX_SHARES) - shares = MAX_SHARES; + shares = clamp(shares, scale_load(MIN_SHARES), scale_load(MAX_SHARES)); mutex_lock(&shares_mutex); if (tg->shares == shares) ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-07-01 15:15 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-06-04 9:29 [patch] sched, cgroups: fix MIN_SHARES on 64 bit boxen Mike Galbraith 2011-06-04 11:24 ` Peter Zijlstra 2011-06-04 13:03 ` Mike Galbraith 2011-07-01 15:15 ` [tip:sched/urgent] sched, cgroups: Fix MIN_SHARES on 64-bit boxen tip-bot for Mike Galbraith
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.