Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sat, 10 Jul 2004 01:50:17 +0200 | From | Andrea Arcangeli <> | Subject | Re: [announce] [patch] Voluntary Kernel Preemption Patch |
| |
On Fri, Jul 09, 2004 at 08:51:05PM +0100, Christoph Hellwig wrote: > > unlike the lowlatency patches, this patch doesn't add a lot of new > > scheduling points to the source code, it rather reuses a rich but > > currently inactive set of scheduling points that already exist in the > > 2.6 tree: the might_sleep() debugging checks. Any code point that does > > might_sleep() is in fact ready to sleep at that point. So the patch > > activates these debugging checks to be scheduling points. This reduces > > complexity and impact quite significantly. > > I don't think this is a good idea. Just because a function might sleep > it doesn't mean it should sleep. I'd rather add the might_sleep() to > cond_resched() and replace the former with the latter in the cases where > it makes sense.
agreed. might_sleep() just like BUG() can be defined to noop.
cond_resched() is the API to use.
the other bad thing is that there is no point for the sysctl (in 2.4 that made no sense at all too, yeah it only makes sense for benchmarking easily w/ and w/o the feature but it must be optimized away at the very least with a config option for production), if need_resched is set we _must_ schedule no matter what (a sysctl can only introduce a bug if something). If we spend any cpu checking the sysctl, we should instead spend such cpu to check need_resched in the first place.
The rest is of course very welcome, but you should remove all the pollution from the patch to make it mergeable.
Just convert all those might to cond_resched() and remove all the superflous volountary stuff and config options.
As worse you can leave a single config option LOW_RESCHEDULE_OVERHEAD with PREEMPT=n, that could remove some cond_resched() from an extremely fast path if you're concerned about adding branches in some critical point, but you really seem not concerned since with CONFIG_PREEMPT_VOLUNTARY=y (the only way to enable it) you even _waste_ cpu on these paths to check a worthless sysctl that can only introduce bugs at runtime since it overrides the wishes of the scheduler.
If scheduler is bad fix the scheduler, but as soon as need_resched is set no sysctl must be allowed to mask the wishes of the scheduler. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |