On Wed, Oct 12, 2011 at 05:51:33PM +0000, Miod Vallat wrote:
> > Hi people,
>
> [...]
>
> > I'm not aiming for a "yeah, nice, we'll merge it" on this, but rather
> > for suggestions whether it's worth anyones time to pursue this further.
>
> This is interesting, there might be a good outcome of this diff
> eventually. However as of now:
> - you have removed the bunch of code which tries to make sure processes
> do not hop between cpus unless there is a gain in doing so, on MP
> kernels. Did you try your diff on a MP kernel?
[...]
Yes. Almost all builds I have done so far were MP kernels, which I run
almost all the time (I switch to un-patched kernels for benchmarks
only). I'll do a benchmark with an SP build though to check whether
there are any negative impacts.
> - non-tabbed indent is evil. And makes the diff harder to read.
[...]
I will clean that up.
> - I see many locks removed in there, and moving information to the proc
> structure does not explain all of them. This gives a bad gut feeling
> about the behaviour of this on MP kernels. A really bad gut feeling.
[...]
Works fine with two cores, but I can see where that feeling comes from.
I'll add some more explanations to my changes.
> - you have changed the roundrobin duration from (hz / 10 to (hz / 100).
> However, this computation yields zero on platforms where hz < 100,
> which is not a good idea. The only assumptions you can make about hz
> is that 50 <= hz <= 1024.
[...]
Good point. I'll change that.
> - you removed the enforcement of RLIMIT_CPU. No way. Over my dead body.
> - I also don't really like switching from NQS run queues to a single
> one. I understand you need this to be able to compare the deadlines
> with only one queue walk, but this can become expensive with many
> processes in the runqueue...
[...]
Right. I'm planning to move this to a real priority queue via some sort
of heap, so lookups and insertions can be done in O(log n) instead of
O(n).
> - your priority computation are unexplained magic. What does 20 mean?
> Shouldn't at least one of them be derived from `hz'? Why << 1 versus
> << 5? Are you sure the values you are computed are always within the
> bounds you are expecting? Why isn't there any comment about these
> computations?
[...]
Deriving them from hz didn't cross my mind but you are right of course.
I chose the << 5 because that makes for a relatively linear increase in
the deadlines while not overflowing the value of .tv_usec. I'll add some
explanations though.
>
> Miod
>
Thanks a lot for your time and suggestions.
--
Gregor Best
[demime 1.01d removed an attachment of type application/pgp-signature]
