[ovs-dev] [PATCH 2/2] datapath: Add loop detection for RT kernels.
Ben Pfaff
blp at nicira.com
Mon Oct 25 17:01:05 UTC 2010
On Fri, Oct 22, 2010 at 04:34:14PM -0700, Jesse Gross wrote:
> Our normal loop detection is fairly lightweight but requires
> disabling preemption while packet processing takes place. On
> RT kernels this isn't acceptable and interacts badly with spinlocks,
> so we can't use it. This implements a form of pseudo thread local
> storage and uses that instead as it is capable of tracking a thread
> across CPU migrations. It is more expensive than the preemption
> disabled version, so we continue to use that on non-RT kernels.
>
> Signed-off-by: Jesse Gross <jesse at nicira.com>
I assume that we would revert this before submitting upstream?
Is realtime with NR_CPUS==1 an interesting special case? We could just
use the non-RT implementation in that case, I think.
Softirqs are always threaded on RT, right? So that means that we'd
always have a task_struct. RT adds an "extra_flags" member to
task_struct but only uses two bits of it. We could abuse other bits as
a counter.
Does anything actually guarantee that NR_CPUS is a power of 2? Writing
#define TASK_HASH_BUCKETS NR_CPUS and then "& (TASK_HASH_BUCKETS - 1)"
means that we depend on such a guarantee.
I think that task_hash_lock should be static.
Is there some reason to kzalloc task_hash[] instead of declaring it as a
static array of NR_CPU members? Maybe this is a kernel programming rule
that I don't know about.
The { and } for the "if" inside find_task() are not aligned properly.
The } should be one tab stop farther right.
It's too bad that two atomic ops (atomic_inc_not_zero(),
atomic_dec_and_test()) are needed for something thread-local.
I don't see any actual problems with this, so with the above comments:
Acked-by: Ben Pfaff <blp at nicira.com>
More information about the dev
mailing list