From 9d73c9cac76ba557fdac4a89c1b7eafe132b85a3 Mon Sep 17 00:00:00 2001 From: Pravin B Shelar Date: Tue, 17 Dec 2013 15:43:30 -0800 Subject: [PATCH] datapath: Fix deadlock during stats update. Stats-read needs to lock stats but same lock is taken in stats update in irq context. Therefore it needs to disable irq to avoid following deadlock :- BUG: soft lockup - CPU#1 stuck for 23s! [ovs-vswitchd:1425] CPU 1 Pid: 1425, comm: ovs-vswitchd Tainted: G O 3.2.39-server-nn23 #1 VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform RIP: 0010:[] [] __ticket_spin_lock+0x22/0x30 RSP: 0018:ffff88003fd03b30 EFLAGS: 00000297 RAX: 0000000000000001 RBX: 0000000000000000 RCX: 0000000000000050 RDX: 0000000000000002 RSI: ffff88003d0a9900 RDI: ffff88003ae19598 RBP: ffff88003fd03b30 R08: 0000000000000000 R09: ffff88003ad44048 R10: 0000000000000001 R11: 0000000000000001 R12: ffff88003fd03aa8 R13: ffffffff8164e5de R14: ffff88003fd03b30 R15: ffff88003ae19580 FS: 00007ffb0b428940(0000) GS:ffff88003fd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f3c0ef94000 CR3: 00000000250e2000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process ovs-vswitchd (pid: 1425, threadinfo ffff88002514a000, task ffff8800250aae00) Stack: ffff88003fd03b40 ffffffff8164596e ffff88003fd03b70 ffffffffa027622d ffff88003d0a9900 ffffe8ffffd03800 ffff8800297f5a80 ffff88003fd03ba8 ffff88003fd03c60 ffffffffa02759af ffff88003fd03de0 ffff88003fd03e4c Call Trace: [] _raw_spin_lock+0xe/0x20 [] ovs_flow_stats_update+0x5d/0x100 [openvswitch] [] ovs_dp_process_received_packet+0x8f/0x130 [openvswitch] [] ovs_vport_receive+0x2a/0x30 [openvswitch] [] netdev_frame_hook+0xb8/0x120 [openvswitch] [] ? free_port_rcu+0x30/0x30 [openvswitch] [] __netif_receive_skb+0x1c8/0x620 [] netif_receive_skb+0x80/0x90 [] ? ksize+0x1c/0xc0 [] napi_skb_finish+0x50/0x70 [] napi_gro_receive+0xf5/0x140 [] vmxnet3_rq_rx_complete+0x51e/0x7c0 [vmxnet3] [] ? nommu_map_sg+0xe0/0xe0 [] vmxnet3_poll_rx_only+0x45/0xc0 [vmxnet3] [] net_rx_action+0x134/0x290 [] ? __ticket_spin_lock+0xd/0x30 [] __do_softirq+0xa8/0x210 [] ? _raw_spin_lock+0xe/0x20 [] call_softirq+0x1c/0x30 [] do_softirq+0x65/0xa0 [] irq_exit+0x8e/0xb0 [] do_IRQ+0x63/0xe0 [] common_interrupt+0x6e/0x6e ----------- Bug #21853 Reported-by: Pawan Shukla Signed-off-by: Pravin B Shelar Acked-by: Jesse Gross --- AUTHORS | 1 + datapath/flow.c | 53 +++++++++++++++++++++++++++++-------------------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/AUTHORS b/AUTHORS index 70850b28b..7539b1af3 100644 --- a/AUTHORS +++ b/AUTHORS @@ -209,6 +209,7 @@ Niklas Andersson nandersson@nicira.com Padmanabhan Krishnan kprad1@yahoo.com Pankaj Thakkar thakkar@nicira.com Paulo Cravero pcravero@as2594.net +Pawan Shukla shuklap@vmware.com Peter Balland peter@nicira.com Peter Phaal peter.phaal@inmon.com Prabina Pattnaik Prabina.Pattnaik@nechclst.in diff --git a/datapath/flow.c b/datapath/flow.c index 9b3d3a753..8be380182 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -87,17 +87,25 @@ void ovs_flow_stats_update(struct sw_flow *flow, struct sk_buff *skb) spin_unlock(&stats->lock); } -static void stats_read(struct flow_stats *stats, +static void stats_read(struct flow_stats *stats, bool lock_bh, struct ovs_flow_stats *ovs_stats, unsigned long *used, __be16 *tcp_flags) { - spin_lock(&stats->lock); + if (lock_bh) + spin_lock_bh(&stats->lock); + else + spin_lock(&stats->lock); + if (time_after(stats->used, *used)) *used = stats->used; *tcp_flags |= stats->tcp_flags; ovs_stats->n_packets += stats->packet_count; ovs_stats->n_bytes += stats->byte_count; - spin_unlock(&stats->lock); + + if (lock_bh) + spin_unlock_bh(&stats->lock); + else + spin_unlock(&stats->lock); } void ovs_flow_stats_get(struct sw_flow *flow, struct ovs_flow_stats *ovs_stats, @@ -110,33 +118,38 @@ void ovs_flow_stats_get(struct sw_flow *flow, struct ovs_flow_stats *ovs_stats, memset(ovs_stats, 0, sizeof(*ovs_stats)); if (!flow->stats.is_percpu) { - stats_read(flow->stats.stat, ovs_stats, used, tcp_flags); + stats_read(flow->stats.stat, true, ovs_stats, used, tcp_flags); } else { cur_cpu = get_cpu(); + for_each_possible_cpu(cpu) { struct flow_stats *stats; - - if (cpu == cur_cpu) - local_bh_disable(); + bool lock_bh; stats = per_cpu_ptr(flow->stats.cpu_stats, cpu); - stats_read(stats, ovs_stats, used, tcp_flags); - - if (cpu == cur_cpu) - local_bh_enable(); + lock_bh = (cpu == cur_cpu); + stats_read(stats, lock_bh, ovs_stats, used, tcp_flags); } put_cpu(); } } -static void stats_reset(struct flow_stats *stats) +static void stats_reset(struct flow_stats *stats, bool lock_bh) { - spin_lock(&stats->lock); + if (lock_bh) + spin_lock_bh(&stats->lock); + else + spin_lock(&stats->lock); + stats->used = 0; stats->packet_count = 0; stats->byte_count = 0; stats->tcp_flags = 0; - spin_unlock(&stats->lock); + + if (lock_bh) + spin_unlock_bh(&stats->lock); + else + spin_unlock(&stats->lock); } void ovs_flow_stats_clear(struct sw_flow *flow) @@ -144,19 +157,15 @@ void ovs_flow_stats_clear(struct sw_flow *flow) int cpu, cur_cpu; if (!flow->stats.is_percpu) { - stats_reset(flow->stats.stat); + stats_reset(flow->stats.stat, true); } else { cur_cpu = get_cpu(); for_each_possible_cpu(cpu) { + bool lock_bh; - if (cpu == cur_cpu) - local_bh_disable(); - - stats_reset(per_cpu_ptr(flow->stats.cpu_stats, cpu)); - - if (cpu == cur_cpu) - local_bh_enable(); + lock_bh = (cpu == cur_cpu); + stats_reset(per_cpu_ptr(flow->stats.cpu_stats, cpu), lock_bh); } put_cpu(); } -- 2.20.1