2004-01-23 06:31:59

by Martin J. Bligh

[permalink] [raw]
Subject: [PATCH] clarify find_busiest_group

Fix a minor nit with the find_busiest_group code. No functional change,
but makes the code simpler and clearer. This patch does two things ...
adds some more expansive comments, and removes this if clause:

if (*imbalance < SCHED_LOAD_SCALE
&& max_load - this_load > SCHED_LOAD_SCALE)
*imbalance = SCHED_LOAD_SCALE;

If we remove the scaling factor, we're basically conditionally doing:

if (*imbalance < 1)
*imbalance = 1;

Which is pointless, as the very next thing we do is to remove the scaling
factor, rounding up to the nearest integer as we do:

*imbalance = (*imbalance + SCHED_LOAD_SCALE - 1) >> SCHED_LOAD_SHIFT;

Thus the if statement is redundant, and only makes the code harder to read ;-)

M.

diff -aurpN -X /home/fletch/.diff.exclude mm5/kernel/sched.c mm5-find_busiest_group/kernel/sched.c
--- mm5/kernel/sched.c Wed Jan 21 22:07:11 2004
+++ mm5-find_busiest_group/kernel/sched.c Thu Jan 22 22:21:22 2004
@@ -1440,11 +1440,16 @@ nextgroup:
if (idle == NOT_IDLE && 100*max_load <= domain->imbalance_pct*this_load)
goto out_balanced;

- /* Take the minimum possible imbalance. */
+ /*
+ * We're trying to get all the cpus to the average_load, so we don't
+ * want to push ourselves above the average load, nor do we wish to
+ * reduce the max loaded cpu below the average load, as either of these
+ * actions would just result in more rebalancing later, and ping-pong
+ * tasks around. Thus we look for the minimum possible imbalance.
+ */
*imbalance = min(max_load - avg_load, avg_load - this_load);
- if (*imbalance < SCHED_LOAD_SCALE
- && max_load - this_load > SCHED_LOAD_SCALE)
- *imbalance = SCHED_LOAD_SCALE;
+
+ /* Get rid of the scaling factor now, rounding *up* as we divide */
*imbalance = (*imbalance + SCHED_LOAD_SCALE - 1) >> SCHED_LOAD_SHIFT;

if (*imbalance == 0) {


2004-01-23 06:54:49

by Nick Piggin

[permalink] [raw]
Subject: Re: [PATCH] clarify find_busiest_group



Martin J. Bligh wrote:

>Fix a minor nit with the find_busiest_group code. No functional change,
>but makes the code simpler and clearer. This patch does two things ...
>adds some more expansive comments, and removes this if clause:
>
> if (*imbalance < SCHED_LOAD_SCALE
> && max_load - this_load > SCHED_LOAD_SCALE)
> *imbalance = SCHED_LOAD_SCALE;
>
>If we remove the scaling factor, we're basically conditionally doing:
>
> if (*imbalance < 1)
> *imbalance = 1;
>
>Which is pointless, as the very next thing we do is to remove the scaling
>factor, rounding up to the nearest integer as we do:
>
> *imbalance = (*imbalance + SCHED_LOAD_SCALE - 1) >> SCHED_LOAD_SHIFT;
>
>Thus the if statement is redundant, and only makes the code harder to read ;-)
>

Thanks Martin,
You are right. The redundant code was left over from an older
version. Thanks for the comments too.

Andrew would you like me to take small changes like this and
feed them to you, or are you happy enough to pick them up?
No doubt there will be a a few more.


2004-01-23 07:02:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] clarify find_busiest_group

Nick Piggin <[email protected]> wrote:
>
> Andrew would you like me to take small changes like this and
> feed them to you, or are you happy enough to pick them up?

I scooped that one up. The main outstanding thing is Rusty's largeish
patch.

<looks>

Time for -mm2, methinks.

2004-01-23 12:43:46

by Ed Tomlinson

[permalink] [raw]
Subject: vts have stopped working here

Hi

Is anyone else having problems with vt(s)? I can switch between X and vt 1 without
problems. Trying to use any of the other vt(s) fails.

A+C+F1 flips from X to vt1
A+F2 flips to vt7 (x)
A+C+F2 from X does nothing

In my logs there are messages about init spawing too fast. Suspect that these are
the processes for the Vt(s) started with:

2:23:respawn:/sbin/getty 38400 tty2

etc.

Kernel is 2.6.2-rc2, dist is debian unstable with X from experimental.

TIA,
Ed Tomlinson

2004-01-23 13:05:34

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: vts have stopped working here

On Fri, Jan 23, 2004 at 07:43:38AM -0500, Ed Tomlinson wrote:

> Is anyone else having problems with vt(s)? I can switch between X and vt 1 without
> problems. Trying to use any of the other vt(s) fails.
>
> A+C+F1 flips from X to vt1
> A+F2 flips to vt7 (x)
> A+C+F2 from X does nothing
>
> In my logs there are messages about init spawing too fast. Suspect that these are
> the processes for the Vt(s) started with:
>
> 2:23:respawn:/sbin/getty 38400 tty2

Interesting. The vt's don't exist until something writes to them. So
most likely X is running on vt2 in your case. As to why the processes
keep dying - no idea.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2004-01-23 15:05:49

by Ed Tomlinson

[permalink] [raw]
Subject: Re: vts have stopped working here

On January 23, 2004 08:05 am, Vojtech Pavlik wrote:
> On Fri, Jan 23, 2004 at 07:43:38AM -0500, Ed Tomlinson wrote:
> > Is anyone else having problems with vt(s)? I can switch between X and vt
> > 1 without problems. Trying to use any of the other vt(s) fails.
> >
> > A+C+F1 flips from X to vt1
> > A+F2 flips to vt7 (x)
> > A+C+F2 from X does nothing
> >
> > In my logs there are messages about init spawing too fast. Suspect that
> > these are the processes for the Vt(s) started with:
> >
> > 2:23:respawn:/sbin/getty 38400 tty2
>
> Interesting. The vt's don't exist until something writes to them. So
> most likely X is running on vt2 in your case. As to why the processes
> keep dying - no idea.

No. X is running on vt7. Selecting F2,F3,F4,F5,F6 all get you to the X
screen, and these are the ids init complains about in the log. Interestingly
this happens on two boxes (K6-III 400 via, P3-1.4G Intel).

Ed