Skip to contentSkip to conversation

net: disable preemption before call smp_processor_id()

PATCH Applied Changli Gao 3 participants 4 messages Aug 7, 2010 – Aug 8, 2010

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Changli Gao

author
#0 lore raw

PATCH 1/1 net: disable preemption before call smp_processor_id()

Although netif_rx() isn't expected to be called in process context with preemption enabled, it'd better handle this case. And this is why get_cpu() is used in the non-RPS #ifdef branch. If tree RCU is selected, rcu_read_lock() won't disable preemption, so preempt_disable() should be called explictly.

Signed-off-by Changli Gao <[email protected]>
---
 net/core/dev.c |    2 ++
 1 file changed, 2 insertions(+)
net/core/dev.c +2 -0
index 2b50896..1ae6543 100644
@@ -2517,6 +2517,7 @@ int netif_rx(struct sk_buff *skb)
 		struct rps_dev_flow voidflow, *rflow = &voidflow;
 		int cpu;
 
+		preempt_disable();
 		rcu_read_lock();
 
 		cpu = get_rps_cpu(skb->dev, skb, &rflow);

@@ -2526,6 +2527,7 @@ int netif_rx(struct sk_buff *skb)
 		ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
 
 		rcu_read_unlock();
+		preempt_enable();
 	}
 #else
 	{
maintainer Applied
#1 lore raw

From: Changli Gao <[email protected]>
Date: Sat, 7 Aug 2010 17:26:02 +0800

Quoted 7 lines

Although netif_rx() isn't expected to be called in process context with preemption enabled, it'd better handle this case. And this is why get_cpu() is used in the non-RPS #ifdef branch. If tree RCU is selected, rcu_read_lock() won't disable preemption, so preempt_disable() should be called explictly.

Signed-off-by Changli Gao <[email protected]>

Applied, thanks Changli.

Paul, perhaps the comment above rcu_read_lock()'s definition should be updated in rcupdate.h? It says blocking is not allowed inside of a read side critical section, but obviously with tree-rcu that is not the case.

Either we should add a mention of tree-rcu's semantics or just remote this part of the comment altogether.

#2 lore raw
Quoted 13 lines · David Miller

From: Changli Gao <[email protected]>
Date: Sat, 7 Aug 2010 17:26:02 +0800

Quoted 7 lines

Although netif_rx() isn't expected to be called in process context with preemption enabled, it'd better handle this case. And this is why get_cpu() is used in the non-RPS #ifdef branch. If tree RCU is selected, rcu_read_lock() won't disable preemption, so preempt_disable() should be called explictly.

Signed-off-by Changli Gao <[email protected]>

Applied, thanks Changli.

Quoted 7 lines

Paul, perhaps the comment above rcu_read_lock()'s definition should be updated in rcupdate.h? It says blocking is not allowed inside of a read side critical section, but obviously with tree-rcu that is not the case.

Either we should add a mention of tree-rcu's semantics or just remote this part of the comment altogether.

Good point, that last sentence is quite obsolete. It also survived a recent cleanup. :-/

If I cover preemptible RCU's semantics, a first cut comes out like this:

* In non-preemptible RCU implementations (TREE_RCU and TINY_RCU), it
* is illegal to block while in an RCU read-side critical section. In
* preemptible RCU implementations (TREE_PREEMPT_RCU and TINY_PREEMPT_RCU)
* in CONFIG_PREEMPT kernel builds, RCU read-side critical sections may
* be preempted, but explicit blocking is illegal. Finally, in preemptible
* RCU implementations in real-time (CONFIG_PREEMPT_RT) kernel builds,
* RCU read-side critical sections may be preempted and they may also
* block, but only when acquiring spinlocks that are subject to priority
* inheritance.

Does that seem reasonable?

Thanx, Paul

maintainer
#3 lore raw

From: "Paul E. McKenney" <[email protected]>
Date: Sat, 7 Aug 2010 21:57:51 -0700

Quoted 12 lines

If I cover preemptible RCU's semantics, a first cut comes out like this:

* In non-preemptible RCU implementations (TREE_RCU and TINY_RCU), it
* is illegal to block while in an RCU read-side critical section. In
* preemptible RCU implementations (TREE_PREEMPT_RCU and TINY_PREEMPT_RCU)
* in CONFIG_PREEMPT kernel builds, RCU read-side critical sections may
* be preempted, but explicit blocking is illegal. Finally, in preemptible
* RCU implementations in real-time (CONFIG_PREEMPT_RT) kernel builds,
* RCU read-side critical sections may be preempted and they may also
* block, but only when acquiring spinlocks that are subject to priority
* inheritance.

Quoted 1 line

Does that seem reasonable?

Sounds good to me.