On Mon, Sep 23, 2013 at 11:20 PM, Viresh Kumar <viresh.ku...@linaro.org> wrote:
> On 24 September 2013 02:00, Jesse Gross <je...@nicira.com> wrote:
>> Search net/core/dev.c for RECURSION_LIMIT.
>
> Ahh, thanks for the pointers..
>
> This stuff was added in 2.6.37 And you said this earlier:
>
> "This loop checker is just compatibility code since the exact same
> logic exists in net/core/dev.c on newer kernels."
>
> So does it mean that people working on kernels > 2.6.37 don't actually
> need the loop checker code from OVS? And if has to be fixed for RT
> then it must be fixed in net/core/dev.c alone (For latest kernels)??
Neither loop checker is perfect and it's possible to get both false
positives and negatives with either one. The OVS loop checker is
slightly more aggressive due to the use cases that people tend to use
it with (mostly a lot of tunnels). However, they both operate on the
same principle and over the same sets of conditions. They are also
both just heuristic sanity checks that aren't strictly required.
> Now that code in dev.c does this: rcu_read_lock_bh()... And its
> implementation is:
>
> static inline void rcu_read_lock_bh(void)
> {
> local_bh_disable();
> #ifdef CONFIG_PREEMPT_RT_FULL
> rcu_read_lock();
> #else
> __acquire(RCU_BH);
> rcu_lock_acquire(&rcu_bh_lock_map);
> rcu_lockdep_assert(!rcu_is_cpu_idle(),
> "rcu_read_lock_bh() used illegally while idle");
> #endif
> }
>
> And rcu_read_lock() disables preemption (Atleast when
> CONFIG_PREEMPT_RCU isn't enabled)... And which would
> safeguard our counters from being accessed simultaneously..
Yes and local_bh_disable() will disable preemption even for the cases
where CONFIG_PREEMPT_RCU is set. That means that in the current code
there are neither interrupts nor preemption for all packet processing.
> And so that code wouldn't break for RT usecase..
I'm not sure that follows. In the RT world, all of that basically gets
converted into migrate_disable().
I looked into the handling of per-CPU on RT kernels some more and
basically they assume that there is a lock protecting the data
(possibly per-CPU). That is true in the case of loop checker in
net/core/dev.c but not in the out-of-tree OVS code or in any of the
stats tracking. So you would need some variation of your original
patch.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
