21 Jul
2004
21 Jul
'04
7:49 p.m.
* Scott Wood <scott(a)timesys.com> wrote:
what high/low semantics do you mean, other than the ordering of softirq
sources? (which is currently implemented via the __do_softirq() loop
first looking at the highest prio softirq.) So splitting up ksoftirqd
into two pieces seems like a separate issue.
local_bh_disable() excludes all softirq processing. This means that such
a section must not be preempted. E.g. the networking layer manipulates
per-CPU lists from such sections, if you remove local_bh_disable() then
from the middle of such a section we could preempt into ksoftirqd which
would break the code.
ok - sorry about the misattribution!
Ingo
we already
'daemonize' softirqs in the stock kernel if the load is high
enough. (this is what ksoftirqd does) So the only question is a tunable
to make this deferring of softirq load mandatory. Yarroll's patch is
quite complex, i dont think that is necessary.
What aspects of it do you find unnecessary? The second thread is
needed to maintain the current high/low priority semantics (without
that, you'll either starve regular tasks with a lot of softirqs, or
starve softirqs with a busy userspace, depending on how you set the
priority of the softirq thread). It also has at
least one
conceptual problem, the NOP-ing of local_bh_disable/enable in case of
CONFIG_SOFTIRQ_THREADS is clearly wrong. Yarroll?
Why is it "clearly wrong"? As far as I can tell, the only legitimate
use of it currently is to protect against deadlock (as in
spin_lock_bh()), which is not an issue if all softirqs run from a
thread. There's also the possibility of code relying on it
also being
preempt_disable(); if that's happening, it could be left alone (though
IMHO it'd be better if such code made its dependence on such behavior
explicit), with preempt_disable() being the only real effect.
yes, that's how softirqs are used. The patch changes these semantics.
I've added
a very simple solution to daemonize softirqs similar to
Yarroll's patch to the -H5 version of voluntary-preempt:
BTW, it was my patch; Yarroll only submitted it to the list (as he
stated at the time).