public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox