* [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 [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®