[PATCH 0/1] jump_label logic fixing in dev.c

[PATCH 0/1] jump_label logic fixing in dev.c

[igorm]

From: Igor Maravic <igorm@etf.rs>

Hi all,
I had the problem with this specific patch:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commitdiff;h=588f033075d8c7efe28695402114eab3f9da47c4;hp=66846048f55c6c05a4c46c2daabb773173f8f28d
Problem was occurring when I would ping unreachable host and then, after some while,
I would type Ctrl+C. This is the Call Trace:

------------
Nov 25 14:34:41 igortest kernel: [  102.264227] BUG: sleeping function called from invalid context at kernel/mutex.c:271
Nov 25 14:34:41 igortest kernel: [  102.264273] in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: swapper
Nov 25 14:34:41 igortest kernel: [  102.264305] 1 lock held by swapper/0:
Nov 25 14:34:41 igortest kernel: [  102.264307]  #0:  (&n->timer){+.-...}, at: [<ffffffff8107ce90>] call_timer_fn+0x0/0x340
Nov 25 14:34:41 igortest kernel: [  102.264318] Pid: 0, comm: swapper Not tainted 3.2.0-rc2-net-next-mpls+ #1
Nov 25 14:34:41 igortest kernel: [  102.264320] Call Trace:
Nov 25 14:34:41 igortest kernel: [  102.264321]  <IRQ>  [<ffffffff8104f417>] __might_sleep+0x137/0x1f0
Nov 25 14:34:41 igortest kernel: [  102.264330]  [<ffffffff816b9a2f>] mutex_lock_nested+0x2f/0x370
Nov 25 14:34:41 igortest kernel: [  102.264334]  [<ffffffff810a89fd>] ? trace_hardirqs_off+0xd/0x10
Nov 25 14:34:41 igortest kernel: [  102.264337]  [<ffffffff8109a37f>] ? local_clock+0x6f/0x80
Nov 25 14:34:41 igortest kernel: [  102.264340]  [<ffffffff810a90a5>] ? lock_release_holdtime.part.22+0x15/0x1a0
Nov 25 14:34:41 igortest kernel: [  102.264344]  [<ffffffff81557929>] ? sock_def_write_space+0x59/0x160
Nov 25 14:34:41 igortest kernel: [  102.264347]  [<ffffffff815e936e>] ? arp_error_report+0x3e/0x90
Nov 25 14:34:41 igortest kernel: [  102.264350]  [<ffffffff810969cd>] atomic_dec_and_mutex_lock+0x5d/0x80
Nov 25 14:34:41 igortest kernel: [  102.264354]  [<ffffffff8112fc1d>] jump_label_dec+0x1d/0x50
Nov 25 14:34:41 igortest kernel: [  102.264357]  [<ffffffff81566525>] net_disable_timestamp+0x15/0x20
Nov 25 14:34:41 igortest kernel: [  102.264360]  [<ffffffff81557a75>] sock_disable_timestamp+0x45/0x50
Nov 25 14:34:41 igortest kernel: [  102.264362]  [<ffffffff81557b00>] __sk_free+0x80/0x200
Nov 25 14:34:41 igortest kernel: [  102.264365]  [<ffffffff815578d0>] ? sk_send_sigurg+0x70/0x70
Nov 25 14:34:42 igortest kernel: [  102.264367]  [<ffffffff815e936e>] ? arp_error_report+0x3e/0x90
Nov 25 14:34:42 igortest kernel: [  102.264370]  [<ffffffff81557cba>] sock_wfree+0x3a/0x70
Nov 25 14:34:42 igortest kernel: [  102.264373]  [<ffffffff8155c2b0>] skb_release_head_state+0x70/0x120
Nov 25 14:34:42 igortest kernel: [  102.264376]  [<ffffffff8155c0b6>] __kfree_skb+0x16/0x30
Nov 25 14:34:42 igortest kernel: [  102.264378]  [<ffffffff8155c119>] kfree_skb+0x49/0x170
Nov 25 14:34:42 igortest kernel: [  102.264381]  [<ffffffff815e936e>] arp_error_report+0x3e/0x90
Nov 25 14:34:42 igortest kernel: [  102.264384]  [<ffffffff81575bd9>] neigh_invalidate+0x89/0xc0
Nov 25 14:34:42 igortest kernel: [  102.264387]  [<ffffffff81578dbe>] neigh_timer_handler+0x9e/0x2a0
Nov 25 14:34:42 igortest kernel: [  102.264389]  [<ffffffff81578d20>] ? neigh_update+0x640/0x640
Nov 25 14:34:42 igortest kernel: [  102.264406]  [<ffffffff81073558>] __do_softirq+0xc8/0x3a0
Nov 25 14:34:42 igortest kernel: [  102.264409]  [<ffffffff810a4864>] ? clockevents_program_event+0x74/0x100
Nov 25 14:34:42 igortest kernel: [  102.264411]  [<ffffffff810a5f34>] ? tick_program_event+0x24/0x30
Nov 25 14:34:42 igortest kernel: [  102.264415]  [<ffffffff816c6ffc>] call_softirq+0x1c/0x30
Nov 25 14:34:42 igortest kernel: [  102.264418]  [<ffffffff8101b615>] do_softirq+0xa5/0xe0
Nov 25 14:34:42 igortest kernel: [  102.264420]  [<ffffffff81073bbe>] irq_exit+0xae/0xe0
Nov 25 14:34:42 igortest kernel: [  102.264423]  [<ffffffff816c72de>] smp_apic_timer_interrupt+0x6e/0x99
Nov 25 14:34:42 igortest kernel: [  102.264426]  [<ffffffff816c6473>] apic_timer_interrupt+0x73/0x80
Nov 25 14:34:42 igortest kernel: [  102.264427]  <EOI>  [<ffffffff816c0c20>] ? notifier_call_chain+0x100/0x100
Nov 25 14:34:42 igortest kernel: [  102.264433]  [<ffffffff810400ab>] ? native_safe_halt+0xb/0x10
Nov 25 14:34:42 igortest kernel: [  102.264436]  [<ffffffff810ac86d>] ? trace_hardirqs_on+0xd/0x10
Nov 25 14:34:42 igortest kernel: [  102.264439]  [<ffffffff8102202a>] default_idle+0x4a/0x290
Nov 25 14:34:42 igortest kernel: [  102.264441]  [<ffffffff81022338>] amd_e400_idle+0xc8/0x160
Nov 25 14:34:42 igortest kernel: [  102.264444]  [<ffffffff81018239>] cpu_idle+0xe9/0x150
Nov 25 14:34:42 igortest kernel: [  102.264447]  [<ffffffff81684cce>] rest_init+0xd2/0xe4
Nov 25 14:34:42 igortest kernel: [  102.264450]  [<ffffffff81684bfc>] ? csum_partial_copy_generic+0x16c/0x16c
Nov 25 14:34:42 igortest kernel: [  102.264453]  [<ffffffff81eb5c69>] start_kernel+0x3fb/0x407
Nov 25 14:34:42 igortest kernel: [  102.264456]  [<ffffffff81eb5388>] x86_64_start_reservations+0x132/0x136
Nov 25 14:34:42 igortest kernel: [  102.264459]  [<ffffffff81eb5140>] ? early_idt_handlers+0x140/0x140
Nov 25 14:34:42 igortest kernel: [  102.264461]  [<ffffffff81eb5459>] x86_64_start_kernel+0xcd/0xdc
Nov 25 14:34:42 igortest kernel: [  102.264470]
Nov 25 14:34:42 igortest kernel: [  102.264479] =================================
Nov 25 14:34:42 igortest kernel: [  102.264501] [ INFO: inconsistent lock state ]
Nov 25 14:34:42 igortest kernel: [  102.264523] 3.2.0-rc2-net-next-mpls+ #1
Nov 25 14:34:42 igortest kernel: [  102.264542] ---------------------------------
Nov 25 14:34:42 igortest kernel: [  102.264564] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
Nov 25 14:34:42 igortest kernel: [  102.264594] swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
Nov 25 14:34:42 igortest kernel: [  102.264618]  (jump_label_mutex){+.?.+.}, at: [<ffffffff810969cd>] atomic_dec_and_mutex_lock+0x5d/0x80
Nov 25 14:34:42 igortest kernel: [  102.265911] {SOFTIRQ-ON-W} state was registered at:
Nov 25 14:34:42 igortest kernel: [  102.267182]   [<ffffffff810ac44f>] mark_irqflags+0x15f/0x1c0
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff810ae81b>] __lock_acquire+0x5cb/0xb40
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff810af48a>] lock_acquire+0xaa/0x200
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff816b9a78>] mutex_lock_nested+0x78/0x370
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff8112fa67>] jump_label_lock+0x17/0x20
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff8112fb25>] jump_label_module_notify+0x95/0xe0
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff816c0bb3>] notifier_call_chain+0x93/0x100
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff81099d28>] __blocking_notifier_call_chain+0x78/0xb0
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff81099d76>] blocking_notifier_call_chain+0x16/0x20
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff810be911>] sys_init_module+0x71/0x230
Nov 25 14:34:42 igortest kernel: [  102.268082]   [<ffffffff816c5902>] system_call_fastpath+0x16/0x1b
Nov 25 14:34:42 igortest kernel: [  102.268082] irq event stamp: 356072
Nov 25 14:34:42 igortest kernel: [  102.268082] hardirqs last  enabled at (356072): [<ffffffff81072dc7>] local_bh_enable_ip+0x97/0x100
Nov 25 14:34:42 igortest kernel: [  102.268082] hardirqs last disabled at (356071): [<ffffffff81072d74>] local_bh_enable_ip+0x44/0x100
Nov 25 14:34:42 igortest kernel: [  102.268082] softirqs last  enabled at (355994): [<ffffffff81072263>] _local_bh_enable+0x13/0x20
Nov 25 14:34:42 igortest kernel: [  102.268082] softirqs last disabled at (355995): [<ffffffff816c6ffc>] call_softirq+0x1c/0x30
Nov 25 14:34:42 igortest kernel: [  102.268082]
Nov 25 14:34:42 igortest kernel: [  102.268082] other info that might help us debug this:
Nov 25 14:34:42 igortest kernel: [  102.268082]  Possible unsafe locking scenario:
Nov 25 14:34:42 igortest kernel: [  102.268082]
Nov 25 14:34:42 igortest kernel: [  102.268082]        CPU0
Nov 25 14:34:42 igortest kernel: [  102.268082]        ----
Nov 25 14:34:42 igortest kernel: [  102.268082]   lock(jump_label_mutex);
Nov 25 14:34:42 igortest kernel: [  102.268082]   <Interrupt>
Nov 25 14:34:42 igortest kernel: [  102.268082]     lock(jump_label_mutex);
Nov 25 14:34:42 igortest kernel: [  102.268082]
Nov 25 14:34:42 igortest kernel: [  102.268082]  *** DEADLOCK ***
Nov 25 14:34:42 igortest kernel: [  102.268082]
Nov 25 14:34:42 igortest kernel: [  102.268082] 1 lock held by swapper/0:
Nov 25 14:34:42 igortest kernel: [  102.268082]  #0:  (&n->timer){+.-...}, at: [<ffffffff8107ce90>] call_timer_fn+0x0/0x340
Nov 25 14:34:42 igortest kernel: [  102.268082]
Nov 25 14:34:42 igortest kernel: [  102.268082] stack backtrace:
Nov 25 14:34:42 igortest kernel: [  102.268082] Pid: 0, comm: swapper Not tainted 3.2.0-rc2-net-next-mpls+ #1
Nov 25 14:34:42 igortest kernel: [  102.268082] Call Trace:
Nov 25 14:34:42 igortest kernel: [  102.268082]  <IRQ>  [<ffffffff816a3725>] print_usage_bug+0x164/0x175
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810aa520>] ? print_irq_inversion_bug.part.33+0x1f0/0x1f0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff816a37a2>] mark_lock_irq+0x6c/0x137
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810ac225>] mark_lock+0x125/0x1f0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810ac409>] mark_irqflags+0x119/0x1c0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810ae81b>] __lock_acquire+0x5cb/0xb40
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810af48a>] lock_acquire+0xaa/0x200
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810969cd>] ? atomic_dec_and_mutex_lock+0x5d/0x80
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8169f6b2>] ? dump_stack+0x77/0x80
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff816b9a78>] mutex_lock_nested+0x78/0x370
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810969cd>] ? atomic_dec_and_mutex_lock+0x5d/0x80
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810a89fd>] ? trace_hardirqs_off+0xd/0x10
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8109a37f>] ? local_clock+0x6f/0x80
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810969cd>] ? atomic_dec_and_mutex_lock+0x5d/0x80
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810a90a5>] ? lock_release_holdtime.part.22+0x15/0x1a0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81557929>] ? sock_def_write_space+0x59/0x160
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff815e936e>] ? arp_error_report+0x3e/0x90
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810969cd>] atomic_dec_and_mutex_lock+0x5d/0x80
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8112fc1d>] jump_label_dec+0x1d/0x50
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81566525>] net_disable_timestamp+0x15/0x20
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81557a75>] sock_disable_timestamp+0x45/0x50
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81557b00>] __sk_free+0x80/0x200
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff815578d0>] ? sk_send_sigurg+0x70/0x70
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff815e936e>] ? arp_error_report+0x3e/0x90
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81557cba>] sock_wfree+0x3a/0x70
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8155c2b0>] skb_release_head_state+0x70/0x120
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8155c0b6>] __kfree_skb+0x16/0x30
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8155c119>] kfree_skb+0x49/0x170
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff815e936e>] arp_error_report+0x3e/0x90
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81575bd9>] neigh_invalidate+0x89/0xc0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81578dbe>] neigh_timer_handler+0x9e/0x2a0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81578d20>] ? neigh_update+0x640/0x640
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8107cf1a>] call_timer_fn+0x8a/0x340
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8107ce90>] ? init_timer_deferrable_key+0x30/0x30
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8107d309>] run_timer_softirq+0x139/0x300
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81073511>] ? __do_softirq+0x81/0x3a0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81578d20>] ? neigh_update+0x640/0x640
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81073558>] __do_softirq+0xc8/0x3a0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810a4864>] ? clockevents_program_event+0x74/0x100
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810a5f34>] ? tick_program_event+0x24/0x30
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff816c6ffc>] call_softirq+0x1c/0x30
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8101b615>] do_softirq+0xa5/0xe0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81073bbe>] irq_exit+0xae/0xe0
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff816c72de>] smp_apic_timer_interrupt+0x6e/0x99
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff816c6473>] apic_timer_interrupt+0x73/0x80
Nov 25 14:34:42 igortest kernel: [  102.268082]  <EOI>  [<ffffffff816c0c20>] ? notifier_call_chain+0x100/0x100
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810400ab>] ? native_safe_halt+0xb/0x10
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff810ac86d>] ? trace_hardirqs_on+0xd/0x10
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff8102202a>] default_idle+0x4a/0x290
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81022338>] amd_e400_idle+0xc8/0x160
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81018239>] cpu_idle+0xe9/0x150
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81684cce>] rest_init+0xd2/0xe4
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81684bfc>] ? csum_partial_copy_generic+0x16c/0x16c
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81eb5c69>] start_kernel+0x3fb/0x407
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81eb5388>] x86_64_start_reservations+0x132/0x136
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81eb5140>] ? early_idt_handlers+0x140/0x140
Nov 25 14:34:42 igortest kernel: [  102.268082]  [<ffffffff81eb5459>] x86_64_start_kernel+0xcd/0xdc
------------------
Problem was calling net_disable_timestamp from Interrupt. By reverting netstamp_needed from jump_label_key 
to atomic_t I solved the problem. 
BR
Igor

Igor Maravic (1):
  netstamp_needed shouldn't be jump_label_key

 net/core/dev.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

-- 
1.7.5.4

[PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[igorm]

From: Igor Maravic <igorm@etf.rs>

Problem with setting netstamp_needed as jump_label_key is that
it inc/dec, functions net_enable_timestamp/net_disable_timestamp,
are called from interrupts.

That can cause DEADLOCK, because jump_label_{inc, dec} are using mutex locking,
that may sleep.

Signed-off-by: Igor Maravic <igorm@etf.rs>

:100644 100644 d1f1071... 4d88cac... M	net/core/dev.c

diff --git a/net/core/dev.c b/net/core/dev.c
index d1f1071..4d88cac 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1442,29 +1442,29 @@ int call_netdevice_notifiers(unsigned long val, struct net_device *dev)
 }
 EXPORT_SYMBOL(call_netdevice_notifiers);
 
-static struct jump_label_key netstamp_needed __read_mostly;
+static atomic_t netstamp_needed = ATOMIC_INIT(0);
 
 void net_enable_timestamp(void)
 {
-	jump_label_inc(&netstamp_needed);
+	atomic_inc(&netstamp_needed);
 }
 EXPORT_SYMBOL(net_enable_timestamp);
 
 void net_disable_timestamp(void)
 {
-	jump_label_dec(&netstamp_needed);
+	atomic_dec(&netstamp_needed);
 }
 EXPORT_SYMBOL(net_disable_timestamp);
 
 static inline void net_timestamp_set(struct sk_buff *skb)
 {
 	skb->tstamp.tv64 = 0;
-	if (static_branch(&netstamp_needed))
+	if (atomic_read(&netstamp_needed))
 		__net_timestamp(skb);
 }
 
 #define net_timestamp_check(COND, SKB)			\
-	if (static_branch(&netstamp_needed)) {		\
+	if (atomic_read(&netstamp_needed)) {		\
 		if ((COND) && !(SKB)->tstamp.tv64)	\
 			__net_timestamp(SKB);		\
 	}						\
-- 
1.7.5.4

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Eric Dumazet]

Le lundi 28 novembre 2011 à 21:23 +0100, igorm@etf.rs a écrit :
> From: Igor Maravic <igorm@etf.rs>
> 
> Problem with setting netstamp_needed as jump_label_key is that
> it inc/dec, functions net_enable_timestamp/net_disable_timestamp,
> are called from interrupts.
> 
> That can cause DEADLOCK, because jump_label_{inc, dec} are using mutex locking,
> that may sleep.
> 
> Signed-off-by: Igor Maravic <igorm@etf.rs>
> 

Come on, we can find another way to handle this :)

Its net-next, we are allowed to try to find something better.

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Eric Dumazet]

Le lundi 28 novembre 2011 à 21:33 +0100, Eric Dumazet a écrit :
> Le lundi 28 novembre 2011 à 21:23 +0100, igorm@etf.rs a écrit :
> > From: Igor Maravic <igorm@etf.rs>
> > 
> > Problem with setting netstamp_needed as jump_label_key is that
> > it inc/dec, functions net_enable_timestamp/net_disable_timestamp,
> > are called from interrupts.
> > 
> > That can cause DEADLOCK, because jump_label_{inc, dec} are using mutex locking,
> > that may sleep.
> > 
> > Signed-off-by: Igor Maravic <igorm@etf.rs>
> > 
> 
> Come on, we can find another way to handle this :)
> 
> Its net-next, we are allowed to try to find something better.
> 

Could you test following patch ?

Thanks

[PATCH net-next] net: dont call jump_label_dec from irq context

Igor Maravic reported an error caused by jump_label_dec() being called
from IRQ context :

 BUG: sleeping function called from invalid context at kernel/mutex.c:271
 in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: swapper
 1 lock held by swapper/0:
  #0:  (&n->timer){+.-...}, at: [<ffffffff8107ce90>] call_timer_fn+0x0/0x340
 Pid: 0, comm: swapper Not tainted 3.2.0-rc2-net-next-mpls+ #1
Call Trace:
 <IRQ>  [<ffffffff8104f417>] __might_sleep+0x137/0x1f0
 [<ffffffff816b9a2f>] mutex_lock_nested+0x2f/0x370
 [<ffffffff810a89fd>] ? trace_hardirqs_off+0xd/0x10
 [<ffffffff8109a37f>] ? local_clock+0x6f/0x80
 [<ffffffff810a90a5>] ? lock_release_holdtime.part.22+0x15/0x1a0
 [<ffffffff81557929>] ? sock_def_write_space+0x59/0x160
 [<ffffffff815e936e>] ? arp_error_report+0x3e/0x90
 [<ffffffff810969cd>] atomic_dec_and_mutex_lock+0x5d/0x80
 [<ffffffff8112fc1d>] jump_label_dec+0x1d/0x50
 [<ffffffff81566525>] net_disable_timestamp+0x15/0x20
 [<ffffffff81557a75>] sock_disable_timestamp+0x45/0x50
 [<ffffffff81557b00>] __sk_free+0x80/0x200
 [<ffffffff815578d0>] ? sk_send_sigurg+0x70/0x70
 [<ffffffff815e936e>] ? arp_error_report+0x3e/0x90
 [<ffffffff81557cba>] sock_wfree+0x3a/0x70
 [<ffffffff8155c2b0>] skb_release_head_state+0x70/0x120
 [<ffffffff8155c0b6>] __kfree_skb+0x16/0x30
 [<ffffffff8155c119>] kfree_skb+0x49/0x170
 [<ffffffff815e936e>] arp_error_report+0x3e/0x90
 [<ffffffff81575bd9>] neigh_invalidate+0x89/0xc0
 [<ffffffff81578dbe>] neigh_timer_handler+0x9e/0x2a0
 [<ffffffff81578d20>] ? neigh_update+0x640/0x640
 [<ffffffff81073558>] __do_softirq+0xc8/0x3a0

Since jump_label_{inc|dec} must be called from process context only,
we must defer jump_label_dec() if net_disable_timestamp() is called
from interrupt context.

Reported-by: Igor Maravic <igorm@etf.rs>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/core/dev.c                 |   23 +++++++++++++++++++++++
 net/ipv4/netfilter/ip_queue.c  |    6 ++++--
 net/ipv6/netfilter/ip6_queue.c |    5 ++++-
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8afb244..de5266c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1443,15 +1443,38 @@ int call_netdevice_notifiers(unsigned long val, struct net_device *dev)
 EXPORT_SYMBOL(call_netdevice_notifiers);
 
 static struct jump_label_key netstamp_needed __read_mostly;
+#ifdef HAVE_JUMP_LABEL
+/* We are not allowed to call jump_label_dec() from irq context
+ * If net_disable_timestamp() is called from irq context, defer the
+ * jump_label_dec() calls.
+ */
+static atomic_t netstamp_needed_deferred;
+#endif
 
 void net_enable_timestamp(void)
 {
+#ifdef HAVE_JUMP_LABEL
+	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
+
+	if (deferred) {
+		while (--deferred)
+			jump_label_dec(&netstamp_needed);
+		return;
+	}
+#endif
+	WARN_ON(in_interrupt());
 	jump_label_inc(&netstamp_needed);
 }
 EXPORT_SYMBOL(net_enable_timestamp);
 
 void net_disable_timestamp(void)
 {
+#ifdef HAVE_JUMP_LABEL
+	if (in_interrupt()) {
+		atomic_inc(&netstamp_needed_deferred);
+		return;
+	}
+#endif
 	jump_label_dec(&netstamp_needed);
 }
 EXPORT_SYMBOL(net_disable_timestamp);
diff --git a/net/ipv4/netfilter/ip_queue.c b/net/ipv4/netfilter/ip_queue.c
index e59aabd..a057fe6 100644
--- a/net/ipv4/netfilter/ip_queue.c
+++ b/net/ipv4/netfilter/ip_queue.c
@@ -404,6 +404,7 @@ __ipq_rcv_skb(struct sk_buff *skb)
 	int status, type, pid, flags;
 	unsigned int nlmsglen, skblen;
 	struct nlmsghdr *nlh;
+	bool enable_timestamp = false;
 
 	skblen = skb->len;
 	if (skblen < sizeof(*nlh))
@@ -441,12 +442,13 @@ __ipq_rcv_skb(struct sk_buff *skb)
 			RCV_SKB_FAIL(-EBUSY);
 		}
 	} else {
-		net_enable_timestamp();
+		enable_timestamp = true;
 		peer_pid = pid;
 	}
 
 	spin_unlock_bh(&queue_lock);
-
+	if (enable_timestamp)
+		net_enable_timestamp();
 	status = ipq_receive_peer(NLMSG_DATA(nlh), type,
 				  nlmsglen - NLMSG_LENGTH(0));
 	if (status < 0)
diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c
index e63c397..fb80a23 100644
--- a/net/ipv6/netfilter/ip6_queue.c
+++ b/net/ipv6/netfilter/ip6_queue.c
@@ -405,6 +405,7 @@ __ipq_rcv_skb(struct sk_buff *skb)
 	int status, type, pid, flags;
 	unsigned int nlmsglen, skblen;
 	struct nlmsghdr *nlh;
+	bool enable_timestamp = false;
 
 	skblen = skb->len;
 	if (skblen < sizeof(*nlh))
@@ -442,11 +443,13 @@ __ipq_rcv_skb(struct sk_buff *skb)
 			RCV_SKB_FAIL(-EBUSY);
 		}
 	} else {
-		net_enable_timestamp();
+		enable_timestamp = true;
 		peer_pid = pid;
 	}
 
 	spin_unlock_bh(&queue_lock);
+	if (enable_timestamp)
+		net_enable_timestamp();
 
 	status = ipq_receive_peer(NLMSG_DATA(nlh), type,
 				  nlmsglen - NLMSG_LENGTH(0));

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Stephen Hemminger]

On Mon, 28 Nov 2011 22:16:50 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le lundi 28 novembre 2011 à 21:33 +0100, Eric Dumazet a écrit :
> > Le lundi 28 novembre 2011 à 21:23 +0100, igorm@etf.rs a écrit :
> > > From: Igor Maravic <igorm@etf.rs>
> > > 
> > > Problem with setting netstamp_needed as jump_label_key is that
> > > it inc/dec, functions net_enable_timestamp/net_disable_timestamp,
> > > are called from interrupts.
> > > 
> > > That can cause DEADLOCK, because jump_label_{inc, dec} are using mutex locking,
> > > that may sleep.
> > > 
> > > Signed-off-by: Igor Maravic <igorm@etf.rs>
> > > 
> > 
> > Come on, we can find another way to handle this :)
> > 
> > Its net-next, we are allowed to try to find something better.
> > 
> 
> Could you test following patch ?
> 
> Thanks
> 
> [PATCH net-next] net: dont call jump_label_dec from irq context
> 
> Igor Maravic reported an error caused by jump_label_dec() being called
> from IRQ context :
> 
>  BUG: sleeping function called from invalid context at kernel/mutex.c:271
>  in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: swapper
>  1 lock held by swapper/0:
>   #0:  (&n->timer){+.-...}, at: [<ffffffff8107ce90>] call_timer_fn+0x0/0x340
>  Pid: 0, comm: swapper Not tainted 3.2.0-rc2-net-next-mpls+ #1
> Call Trace:
>  <IRQ>  [<ffffffff8104f417>] __might_sleep+0x137/0x1f0
>  [<ffffffff816b9a2f>] mutex_lock_nested+0x2f/0x370
>  [<ffffffff810a89fd>] ? trace_hardirqs_off+0xd/0x10
>  [<ffffffff8109a37f>] ? local_clock+0x6f/0x80
>  [<ffffffff810a90a5>] ? lock_release_holdtime.part.22+0x15/0x1a0
>  [<ffffffff81557929>] ? sock_def_write_space+0x59/0x160
>  [<ffffffff815e936e>] ? arp_error_report+0x3e/0x90
>  [<ffffffff810969cd>] atomic_dec_and_mutex_lock+0x5d/0x80
>  [<ffffffff8112fc1d>] jump_label_dec+0x1d/0x50
>  [<ffffffff81566525>] net_disable_timestamp+0x15/0x20
>  [<ffffffff81557a75>] sock_disable_timestamp+0x45/0x50
>  [<ffffffff81557b00>] __sk_free+0x80/0x200
>  [<ffffffff815578d0>] ? sk_send_sigurg+0x70/0x70
>  [<ffffffff815e936e>] ? arp_error_report+0x3e/0x90
>  [<ffffffff81557cba>] sock_wfree+0x3a/0x70
>  [<ffffffff8155c2b0>] skb_release_head_state+0x70/0x120
>  [<ffffffff8155c0b6>] __kfree_skb+0x16/0x30
>  [<ffffffff8155c119>] kfree_skb+0x49/0x170
>  [<ffffffff815e936e>] arp_error_report+0x3e/0x90
>  [<ffffffff81575bd9>] neigh_invalidate+0x89/0xc0
>  [<ffffffff81578dbe>] neigh_timer_handler+0x9e/0x2a0
>  [<ffffffff81578d20>] ? neigh_update+0x640/0x640
>  [<ffffffff81073558>] __do_softirq+0xc8/0x3a0
> 
> Since jump_label_{inc|dec} must be called from process context only,
> we must defer jump_label_dec() if net_disable_timestamp() is called
> from interrupt context.
> 
> Reported-by: Igor Maravic <igorm@etf.rs>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/core/dev.c                 |   23 +++++++++++++++++++++++
>  net/ipv4/netfilter/ip_queue.c  |    6 ++++--
>  net/ipv6/netfilter/ip6_queue.c |    5 ++++-
>  3 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 8afb244..de5266c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1443,15 +1443,38 @@ int call_netdevice_notifiers(unsigned long val, struct net_device *dev)
>  EXPORT_SYMBOL(call_netdevice_notifiers);
>  
>  static struct jump_label_key netstamp_needed __read_mostly;
> +#ifdef HAVE_JUMP_LABEL
> +/* We are not allowed to call jump_label_dec() from irq context
> + * If net_disable_timestamp() is called from irq context, defer the
> + * jump_label_dec() calls.
> + */
> +static atomic_t netstamp_needed_deferred;
> +#endif
>  
>  void net_enable_timestamp(void)
>  {
> +#ifdef HAVE_JUMP_LABEL
> +	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
> +
> +	if (deferred) {
> +		while (--deferred)
> +			jump_label_dec(&netstamp_needed);
> +		return;
> +	}
> +#endif
> +	WARN_ON(in_interrupt());
>  	jump_label_inc(&netstamp_needed);
>  }
>  EXPORT_SYMBOL(net_enable_timestamp);
>  
>  void net_disable_timestamp(void)
>  {
> +#ifdef HAVE_JUMP_LABEL
> +	if (in_interrupt()) {
> +		atomic_inc(&netstamp_needed_deferred);
> +		return;
> +	}
> +#endif
>  	jump_label_dec(&netstamp_needed);
>  }
>  EXPORT_SYMBOL(net_disable_timestamp);
> diff --git a/net/ipv4/netfilter/ip_queue.c b/net/ipv4/netfilter/ip_queue.c
> index e59aabd..a057fe6 100644
> --- a/net/ipv4/netfilter/ip_queue.c
> +++ b/net/ipv4/netfilter/ip_queue.c
> @@ -404,6 +404,7 @@ __ipq_rcv_skb(struct sk_buff *skb)
>  	int status, type, pid, flags;
>  	unsigned int nlmsglen, skblen;
>  	struct nlmsghdr *nlh;
> +	bool enable_timestamp = false;
>  
>  	skblen = skb->len;
>  	if (skblen < sizeof(*nlh))
> @@ -441,12 +442,13 @@ __ipq_rcv_skb(struct sk_buff *skb)
>  			RCV_SKB_FAIL(-EBUSY);
>  		}
>  	} else {
> -		net_enable_timestamp();
> +		enable_timestamp = true;
>  		peer_pid = pid;
>  	}
>  
>  	spin_unlock_bh(&queue_lock);
> -
> +	if (enable_timestamp)
> +		net_enable_timestamp();
>  	status = ipq_receive_peer(NLMSG_DATA(nlh), type,
>  				  nlmsglen - NLMSG_LENGTH(0));
>  	if (status < 0)
> diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c
> index e63c397..fb80a23 100644
> --- a/net/ipv6/netfilter/ip6_queue.c
> +++ b/net/ipv6/netfilter/ip6_queue.c
> @@ -405,6 +405,7 @@ __ipq_rcv_skb(struct sk_buff *skb)
>  	int status, type, pid, flags;
>  	unsigned int nlmsglen, skblen;
>  	struct nlmsghdr *nlh;
> +	bool enable_timestamp = false;
>  
>  	skblen = skb->len;
>  	if (skblen < sizeof(*nlh))
> @@ -442,11 +443,13 @@ __ipq_rcv_skb(struct sk_buff *skb)
>  			RCV_SKB_FAIL(-EBUSY);
>  		}
>  	} else {
> -		net_enable_timestamp();
> +		enable_timestamp = true;
>  		peer_pid = pid;
>  	}
>  
>  	spin_unlock_bh(&queue_lock);
> +	if (enable_timestamp)
> +		net_enable_timestamp();
>  
>  	status = ipq_receive_peer(NLMSG_DATA(nlh), type,
>  				  nlmsglen - NLMSG_LENGTH(0));

Why not just change the jump_label_key to a spin_lock? I can't see that it is held
long enough to make mutex of any benefit.

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Eric Dumazet]

Le lundi 28 novembre 2011 à 13:31 -0800, Stephen Hemminger a écrit :

> Why not just change the jump_label_key to a spin_lock? I can't see that it is held
> long enough to make mutex of any benefit.

Because some arches want to use mutex in their
arch_jump_label_transform() or text_poke_smp()

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Eric Dumazet]

Le lundi 28 novembre 2011 à 22:16 +0100, Eric Dumazet a écrit :

> [PATCH net-next] net: dont call jump_label_dec from irq context
> 
> Igor Maravic reported an error caused by jump_label_dec() being called
> from IRQ context :

> ...

> Since jump_label_{inc|dec} must be called from process context only,
> we must defer jump_label_dec() if net_disable_timestamp() is called
> from interrupt context.
> 
> Reported-by: Igor Maravic <igorm@etf.rs>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/core/dev.c                 |   23 +++++++++++++++++++++++
>  net/ipv4/netfilter/ip_queue.c  |    6 ++++--
>  net/ipv6/netfilter/ip6_queue.c |    5 ++++-
>  3 files changed, 31 insertions(+), 3 deletions(-)

By the way, we also can call sock_disable_timestamp() from sk_free()
instead of calling it from __sk_free()

This makes no sense to wait that all in-flight packets were destroyed
before removing sk timestamping.

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 28 Nov 2011 22:16:50 +0100

> [PATCH net-next] net: dont call jump_label_dec from irq context
> 
> Igor Maravic reported an error caused by jump_label_dec() being called
> from IRQ context :
 ...
> Since jump_label_{inc|dec} must be called from process context only,
> we must defer jump_label_dec() if net_disable_timestamp() is called
> from interrupt context.
> 
> Reported-by: Igor Maravic <igorm@etf.rs>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

["Igor Maravić"]

>
> Could you test following patch ?
>

I've just put it to compile. I'l let you know soon as it finishes compiling.

I have one proposition for your patch. To put WARN_ON before in ifdef endif,
because we don't needed if we don't have HAVE_JUMP_LABEL, and also to check it
before using jump_label_dec.

Also I have few questions :)

First - why can't we use spin_lock on jump_label?

I know that You said that is because we are using mutex_lock in
arch_jump_label_transform() or text_poke_smp(), but I don't see why couldn't
we use mutex_lock inside spin_lock.

To me using spin_lock_irqsave and spin_lock_irqrestore does sound like as most logical solution.
With that lock, we would be sure that our mutex_lock in arch_jump_label_transform() or text_poke_smp()
isn't going to be interrupted.
Please correct if I'm wrong.

Also I saw that you put calling of net_enable_timestamp outside of spin_lock_bh
Why?

BR
Igor

[PATCH 1/1] jump_label warning if_interrupt

Move warning on the top of function net_enable_timestamp,
so we would be also warn if we are going to jump_label_dec in interrupt

Signed-off-by: Igor Maravic <igorm@etf.rs>

:100644 100644 45eab03... ef23cf7... M	net/core/dev.c

diff --git a/net/core/dev.c b/net/core/dev.c
index 45eab03..ef23cf7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1453,6 +1453,7 @@ void net_enable_timestamp(void)
 {
 #ifdef HAVE_JUMP_LABEL
 	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
+	WARN_ON(in_interrupt());

 	if (deferred) {
 		while (--deferred)
@@ -1460,7 +1461,6 @@ void net_enable_timestamp(void)
 		return;
 	}
 #endif
-	WARN_ON(in_interrupt());
 	jump_label_inc(&netstamp_needed);
 }
 EXPORT_SYMBOL(net_enable_timestamp);
-- 
1.7.5.4

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Eric Dumazet]

Le mardi 29 novembre 2011 à 10:03 +0100, "Igor Maravić" a écrit :
> >
> > Could you test following patch ?
> >
> 
> I've just put it to compile. I'l let you know soon as it finishes compiling.
> 
> I have one proposition for your patch. To put WARN_ON before in ifdef endif,
> because we don't needed if we don't have HAVE_JUMP_LABEL, and also to check it
> before using jump_label_dec.
> 
> Also I have few questions :)
> 
> First - why can't we use spin_lock on jump_label?
> 
> I know that You said that is because we are using mutex_lock in
> arch_jump_label_transform() or text_poke_smp(), but I don't see why couldn't
> we use mutex_lock inside spin_lock.
> 


Try it, you'll see how bad it is.

> To me using spin_lock_irqsave and spin_lock_irqrestore does sound like as most logical solution.
> With that lock, we would be sure that our mutex_lock in arch_jump_label_transform() or text_poke_smp()
> isn't going to be interrupted.
> Please correct if I'm wrong.
> 
> Also I saw that you put calling of net_enable_timestamp outside of spin_lock_bh
> Why?
> 

Because its the same problem. We want to be allowed to sleep.

> BR
> Igor
> 
> [PATCH 1/1] jump_label warning if_interrupt
> 
> Move warning on the top of function net_enable_timestamp,
> so we would be also warn if we are going to jump_label_dec in interrupt
> 
> Signed-off-by: Igor Maravic <igorm@etf.rs>
> 
> :100644 100644 45eab03... ef23cf7... M	net/core/dev.c
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 45eab03..ef23cf7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1453,6 +1453,7 @@ void net_enable_timestamp(void)
>  {
>  #ifdef HAVE_JUMP_LABEL
>  	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
> +	WARN_ON(in_interrupt());
> 
>  	if (deferred) {
>  		while (--deferred)
> @@ -1460,7 +1461,6 @@ void net_enable_timestamp(void)
>  		return;
>  	}
>  #endif
> -	WARN_ON(in_interrupt());
>  	jump_label_inc(&netstamp_needed);
>  }
>  EXPORT_SYMBOL(net_enable_timestamp);

Its better to have coverage of the test, even on machines with no
HAVE_JUMP_LABEL.

So move the test before the 
#ifdef HAVE_JUMP_LABEL

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

["Igor Maravić"]

> Its better to have coverage of the test, even on machines with no
> HAVE_JUMP_LABEL.
>
> So move the test before the
> #ifdef HAVE_JUMP_LABEL
>

Here is it.

PS. Tested the patch - it works.

[PATCH 1/1] Warning if_interrupt is true

Move warning on the top of function net_enable_timestamp,
so we would be also warn if we are going to use jump_label_dec in interrupt

Signed-off-by: Igor Maravic <igorm@etf.rs>

:100644 100644 45eab03... d358088... M	net/core/dev.c

diff --git a/net/core/dev.c b/net/core/dev.c
index 45eab03..d358088 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1451,16 +1451,18 @@ static atomic_t netstamp_needed_deferred;

 void net_enable_timestamp(void)
 {
+	WARN_ON(in_interrupt());
 #ifdef HAVE_JUMP_LABEL
-	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
-
-	if (deferred) {
-		while (--deferred)
-			jump_label_dec(&netstamp_needed);
-		return;
+	{
+		int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
+
+		if (deferred) {
+			while (--deferred)
+				jump_label_dec(&netstamp_needed);
+			return;
+		}
 	}
 #endif
-	WARN_ON(in_interrupt());
 	jump_label_inc(&netstamp_needed);
 }
 EXPORT_SYMBOL(net_enable_timestamp);
-- 
1.7.5.4

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Eric Dumazet]

Le mardi 29 novembre 2011 à 10:48 +0100, "Igor Maravić" a écrit :

> 
> PS. Tested the patch - it works.
> 
> [PATCH 1/1] Warning if_interrupt is true

Few remarks, since you start to send official patches ;)

- Since its not a serie of several patches, you dont need the 1/1
- Since tree is net-next, not net, you should say it to David
- We need a better title, with subsystem: tag, for example

Here, it can be "net:"

[PATCH net-next] net: check net_enable_timestamp() is not called in irq context

> 
> Move warning on the top of function net_enable_timestamp,
> so we would be also warn if we are going to use jump_label_dec in interrupt
> 
> Signed-off-by: Igor Maravic <igorm@etf.rs>
> 
> :100644 100644 45eab03... d358088... M	net/core/dev.c
> 

Missing "---" separator

Please carefully read Documentation/SubmittingPatches

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 45eab03..d358088 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1451,16 +1451,18 @@ static atomic_t netstamp_needed_deferred;
> 
>  void net_enable_timestamp(void)
>  {
> +	WARN_ON(in_interrupt());
>  #ifdef HAVE_JUMP_LABEL
> -	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
> -
> -	if (deferred) {
> -		while (--deferred)
> -			jump_label_dec(&netstamp_needed);
> -		return;
> +	{
> +		int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
> +
> +		if (deferred) {
> +			while (--deferred)
> +				jump_label_dec(&netstamp_needed);
> +			return;
> +		}
>  	}
>  #endif
> -	WARN_ON(in_interrupt());
>  	jump_label_inc(&netstamp_needed);
>  }
>  EXPORT_SYMBOL(net_enable_timestamp);

Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Igor Maravić]

Sorry,
just send another without using your remarks...
Thaks
Will listen to you in future.
BR
Igor

2011/11/29 Eric Dumazet <eric.dumazet@gmail.com>:
> Le mardi 29 novembre 2011 à 10:48 +0100, "Igor Maravić" a écrit :
>
>>
>> PS. Tested the patch - it works.
>>
>> [PATCH 1/1] Warning if_interrupt is true
>
> Few remarks, since you start to send official patches ;)
>
> - Since its not a serie of several patches, you dont need the 1/1
> - Since tree is net-next, not net, you should say it to David
> - We need a better title, with subsystem: tag, for example
>
> Here, it can be "net:"
>
> [PATCH net-next] net: check net_enable_timestamp() is not called in irq context
>
>>
>> Move warning on the top of function net_enable_timestamp,
>> so we would be also warn if we are going to use jump_label_dec in interrupt
>>
>> Signed-off-by: Igor Maravic <igorm@etf.rs>
>>
>> :100644 100644 45eab03... d358088... M        net/core/dev.c
>>
>
> Missing "---" separator
>
> Please carefully read Documentation/SubmittingPatches
>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 45eab03..d358088 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1451,16 +1451,18 @@ static atomic_t netstamp_needed_deferred;
>>
>>  void net_enable_timestamp(void)
>>  {
>> +     WARN_ON(in_interrupt());
>>  #ifdef HAVE_JUMP_LABEL
>> -     int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
>> -
>> -     if (deferred) {
>> -             while (--deferred)
>> -                     jump_label_dec(&netstamp_needed);
>> -             return;
>> +     {
>> +             int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
>> +
>> +             if (deferred) {
>> +                     while (--deferred)
>> +                             jump_label_dec(&netstamp_needed);
>> +                     return;
>> +             }
>>       }
>>  #endif
>> -     WARN_ON(in_interrupt());
>>       jump_label_inc(&netstamp_needed);
>>  }
>>  EXPORT_SYMBOL(net_enable_timestamp);
>
>
>