* [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. @ 2014-10-13 10:00 Martin Townsend 2014-10-13 10:00 ` Martin Townsend 0 siblings, 1 reply; 16+ messages in thread From: Martin Townsend @ 2014-10-13 10:00 UTC (permalink / raw) To: linux-bluetooth, linux-wpan Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend This patch aims to reduce the amount of copying in the receive path when decompressing by checking for headroom and calling pskb_expand_head if required. Another benefit of this is that the skb pointer passed to the decompression routine will be the same skb after decompression. This will be a step towards freeing the skb within the receive function and not within all the error paths of the decompression routine. Bluetooth and 802.15.4 have been changed to ensure that the skb isn't shared before calling the decompression routine as this is a requirement of pskb_expand_head. v1 -> v2 Use const int for new headroom size. Remove blank line. Use skb_unshare instead of skb_copy_expand. Use consume_skb. v2 -> v3 Remove cases where we are trying to free a NULL skb. v3 -> v4 Now uses skb_share_check to ensure the skb isn't shared before decompressing v4 -> v5 Remove skb_share_check from 802.15.4 lowpan_rcv as it's already done at the top of the function. v5 -> v6 Use skb_cow instead of pskb_expand_head, adjusted commit message to reflect this. Moved skb_share_check to start of recv_pkt function for bluetooth. Martin Townsend (1): 6lowpan: Use skb_cow in IPHC decompression. net/6lowpan/iphc.c | 47 +++++++++++++++++++++-------------------------- net/bluetooth/6lowpan.c | 4 ++++ 2 files changed, 25 insertions(+), 26 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 10:00 [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression Martin Townsend @ 2014-10-13 10:00 ` Martin Townsend 2014-10-13 10:34 ` Alexander Aring ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Martin Townsend @ 2014-10-13 10:00 UTC (permalink / raw) To: linux-bluetooth, linux-wpan Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend Currently there are potentially 2 skb_copy_expand calls in IPHC decompression. This patch replaces this with one call to skb_cow which will check to see if there is enough headroom first to ensure it's only done if necessary and will handle alignment issues for cache. As skb_cow uses pskb_expand_head we ensure the skb isn't shared from bluetooth and ieee802.15.4 code that use the IPHC decompression. Signed-off-by: Martin Townsend <martin.townsend@xsilon.com> --- net/6lowpan/iphc.c | 47 +++++++++++++++++++++-------------------------- net/bluetooth/6lowpan.c | 4 ++++ 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c index 142eef5..747b3cc 100644 --- a/net/6lowpan/iphc.c +++ b/net/6lowpan/iphc.c @@ -174,30 +174,22 @@ static int uncompress_context_based_src_addr(struct sk_buff *skb, static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr, struct net_device *dev, skb_delivery_cb deliver_skb) { - struct sk_buff *new; int stat; - new = skb_copy_expand(skb, sizeof(struct ipv6hdr), skb_tailroom(skb), - GFP_ATOMIC); - kfree_skb(skb); - - if (!new) - return -ENOMEM; - - skb_push(new, sizeof(struct ipv6hdr)); - skb_reset_network_header(new); - skb_copy_to_linear_data(new, hdr, sizeof(struct ipv6hdr)); + skb_push(skb, sizeof(struct ipv6hdr)); + skb_reset_network_header(skb); + skb_copy_to_linear_data(skb, hdr, sizeof(struct ipv6hdr)); - new->protocol = htons(ETH_P_IPV6); - new->pkt_type = PACKET_HOST; - new->dev = dev; + skb->protocol = htons(ETH_P_IPV6); + skb->pkt_type = PACKET_HOST; + skb->dev = dev; raw_dump_table(__func__, "raw skb data dump before receiving", - new->data, new->len); + skb->data, skb->len); - stat = deliver_skb(new, dev); + stat = deliver_skb(skb, dev); - kfree_skb(new); + consume_skb(skb); return stat; } @@ -460,7 +452,7 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev, /* UDP data uncompression */ if (iphc0 & LOWPAN_IPHC_NH_C) { struct udphdr uh; - struct sk_buff *new; + const int needed = sizeof(struct udphdr) + sizeof(hdr); if (uncompress_udp_header(skb, &uh)) goto drop; @@ -468,14 +460,11 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev, /* replace the compressed UDP head by the uncompressed UDP * header */ - new = skb_copy_expand(skb, sizeof(struct udphdr), - skb_tailroom(skb), GFP_ATOMIC); - kfree_skb(skb); - - if (!new) - return -ENOMEM; - - skb = new; + err = skb_cow(skb, needed); + if (unlikely(err)) { + kfree_skb(skb); + return err; + } skb_push(skb, sizeof(struct udphdr)); skb_reset_transport_header(skb); @@ -485,6 +474,12 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev, (u8 *)&uh, sizeof(uh)); hdr.nexthdr = UIP_PROTO_UDP; + } else { + err = skb_cow(skb, sizeof(hdr)); + if (unlikely(err)) { + kfree_skb(skb); + return err; + } } hdr.payload_len = htons(skb->len); diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index c2e0d14..4ebc806 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -316,6 +316,10 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev, if (dev->type != ARPHRD_6LOWPAN) goto drop; + skb = skb_share_check(skb, GFP_ATOMIC); + if (!skb) + goto drop; + /* check that it's our buffer */ if (skb->data[0] == LOWPAN_DISPATCH_IPV6) { /* Copy the packet so that the IPv6 header is -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 10:00 ` Martin Townsend @ 2014-10-13 10:34 ` Alexander Aring 2014-10-13 11:49 ` Jukka Rissanen 2014-10-17 14:15 ` Marcel Holtmann 2 siblings, 0 replies; 16+ messages in thread From: Alexander Aring @ 2014-10-13 10:34 UTC (permalink / raw) To: Martin Townsend Cc: linux-bluetooth, linux-wpan, marcel, jukka.rissanen, Martin Townsend On Mon, Oct 13, 2014 at 11:00:56AM +0100, Martin Townsend wrote: > Currently there are potentially 2 skb_copy_expand calls in IPHC > decompression. This patch replaces this with one call to > skb_cow which will check to see if there is enough headroom > first to ensure it's only done if necessary and will handle > alignment issues for cache. > As skb_cow uses pskb_expand_head we ensure the skb isn't shared from > bluetooth and ieee802.15.4 code that use the IPHC decompression. > > Signed-off-by: Martin Townsend <martin.townsend@xsilon.com> Okay, we don't use the goto drop; below and currently this will always return -EINVAL. You will fix that in your next patches, so it doesn't matter right now. Also this will correct a little bit the errno value. Great job Martin. Acked-by: Alexander Aring <alex.aring@gmail.com> Marcel, please wait for jukka's acked here, then we are sure that we didn't break anything in bluetooth 6lowpan. - Alex ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 10:00 ` Martin Townsend 2014-10-13 10:34 ` Alexander Aring @ 2014-10-13 11:49 ` Jukka Rissanen 2014-10-13 13:10 ` Martin Townsend 2014-10-17 14:15 ` Marcel Holtmann 2 siblings, 1 reply; 16+ messages in thread From: Jukka Rissanen @ 2014-10-13 11:49 UTC (permalink / raw) To: Martin Townsend Cc: linux-bluetooth, linux-wpan, marcel, alex.aring, Martin Townsend Hi Martin, with this v6 I started to see similar issues locking issues as in some earlier patch version. I will try v5 and v4 just to make sure what is going on here. [ 308.736047] ================================= [ 308.736047] [ INFO: inconsistent lock state ] [ 308.736047] 3.17.0-rc1-bt6lowpan #1 Not tainted [ 308.736047] --------------------------------- [ 308.736047] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage. [ 308.736047] kworker/u3:2/404 [HC0[0]:SC0[0]:HE1:SE1] takes: [ 308.736047] (&(&list->lock)->rlock#6){+.?...}, at: [<f82a8d4c>] hci_send_acl+0xac/0x290 [bluetooth] [ 308.736047] {IN-SOFTIRQ-W} state was registered at: [ 308.736047] [<c10915a3>] __lock_acquire+0x6d3/0x1d20 [ 308.736047] [<c109325d>] lock_acquire+0x9d/0x140 [ 308.736047] [<c1889c25>] _raw_spin_lock+0x45/0x80 [ 308.736047] [<f82a8d4c>] hci_send_acl+0xac/0x290 [bluetooth] [ 308.736047] [<f82c9bf0>] l2cap_do_send+0x60/0x100 [bluetooth] [ 308.736047] [<f82cd7c0>] l2cap_chan_send+0x7f0/0x10e0 [bluetooth] [ 308.736047] [<f850d91e>] send_pkt+0x4e/0xa0 [bluetooth_6lowpan] [ 308.736047] [<f850dd20>] bt_xmit+0x3b0/0x770 [bluetooth_6lowpan] [ 308.736047] [<c17742f4>] dev_hard_start_xmit+0x344/0x670 [ 308.736047] [<c17749ad>] __dev_queue_xmit+0x38d/0x680 [ 308.736047] [<c1774caf>] dev_queue_xmit+0xf/0x20 [ 308.736047] [<c177b8b0>] neigh_connected_output+0x130/0x1a0 [ 308.736047] [<c1812a63>] ip6_finish_output2+0x173/0x8c0 [ 308.736047] [<c18182db>] ip6_finish_output+0x7b/0x1b0 [ 308.736047] [<c18184a7>] ip6_output+0x97/0x2a0 [ 308.736047] [<c183a46b>] mld_sendpack+0x5eb/0x650 [ 308.736047] [<c183acc1>] mld_ifc_timer_expire+0x191/0x2f0 [ 308.736047] [<c10ac385>] call_timer_fn+0x85/0x1c0 [ 308.736047] [<c10acb72>] run_timer_softirq+0x192/0x280 [ 308.736047] [<c104fd84>] __do_softirq+0xd4/0x300 [ 308.736047] [<c10049fc>] do_softirq_own_stack+0x2c/0x40 [ 308.736047] [<c1050136>] irq_exit+0x86/0xb0 [ 308.736047] [<c188bd98>] smp_apic_timer_interrupt+0x38/0x50 [ 308.736047] [<c188b6ce>] apic_timer_interrupt+0x32/0x38 [ 308.736047] [<c115e6b2>] kmem_cache_alloc+0x1c2/0x290 [ 308.736047] [<c116a9ae>] create_object+0x2e/0x280 [ 308.736047] [<c187f10c>] kmemleak_alloc+0x3c/0xb0 [ 308.736047] [<c115e693>] kmem_cache_alloc+0x1a3/0x290 [ 308.736047] [<c1178835>] getname_flags+0x25/0x110 [ 308.736047] [<c117d21e>] user_path_at_empty+0x1e/0x80 [ 308.736047] [<c1173317>] SyS_readlinkat+0x57/0x100 [ 308.736047] [<c11733ec>] SyS_readlink+0x2c/0x30 [ 308.736047] [<c188aeb6>] syscall_after_call+0x0/0x4 [ 308.736047] irq event stamp: 37665 [ 308.736047] hardirqs last enabled at (37665): [<c188a065>] _raw_spin_unlock_irqrestore+0x55/0x70 [ 308.736047] hardirqs last disabled at (37664): [<c1889e03>] _raw_spin_lock_irqsave+0x23/0x90 [ 308.736047] softirqs last enabled at (37378): [<c104ff5c>] __do_softirq+0x2ac/0x300 [ 308.736047] softirqs last disabled at (37359): [<c10049fc>] do_softirq_own_stack+0x2c/0x40 [ 308.736047] [ 308.736047] other info that might help us debug this: [ 308.736047] Possible unsafe locking scenario: [ 308.736047] [ 308.736047] CPU0 [ 308.736047] ---- [ 308.736047] lock(&(&list->lock)->rlock#6); [ 308.736047] <Interrupt> [ 308.736047] lock(&(&list->lock)->rlock#6); [ 308.736047] [ 308.736047] *** DEADLOCK *** [ 308.736047] [ 308.736047] 3 locks held by kworker/u3:2/404: [ 308.736047] #0: ("%s"hdev->name#2){.+.+.+}, at: [<c10622c3>] process_one_work+0x113/0x4a0 [ 308.736047] #1: ((&hdev->rx_work)){+.+.+.}, at: [<c10622c3>] process_one_work+0x113/0x4a0 [ 308.736047] #2: (&chan->lock){+.+.+.}, at: [<f82c9179>] l2cap_get_chan_by_dcid+0x89/0x90 [bluetooth] [ 308.736047] [ 308.736047] stack backtrace: [ 308.736047] CPU: 0 PID: 404 Comm: kworker/u3:2 Not tainted 3.17.0-rc1-bt6lowpan #1 [ 308.736047] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 308.736047] Workqueue: hci0 hci_rx_work [bluetooth] [ 308.736047] efe3d780 00000000 efedbb90 c18821c1 c2345c10 efedbbb0 c1880a94 c1ad9264 [ 308.736047] c1ad9201 c1ad95f8 efe3dd94 00000006 c108e0c0 efedbbe0 c108ea8c 00000006 [ 308.736047] efe3d780 efedbffc e21128f4 00000047 00000004 efe3d780 efe3dd98 00000003 [ 308.736047] Call Trace: [ 308.736047] [<c18821c1>] dump_stack+0x4b/0x75 [ 308.736047] [<c1880a94>] print_usage_bug.part.36+0x209/0x213 [ 308.736047] [<c108e0c0>] ? check_usage_forwards+0x110/0x110 [ 308.736047] [<c108ea8c>] mark_lock+0x11c/0x6e0 [ 308.736047] [<c10915e4>] __lock_acquire+0x714/0x1d20 [ 308.736047] [<c1004b6f>] ? dump_trace+0xcf/0x1f0 [ 308.736047] [<c100a5f8>] ? sched_clock+0x8/0x10 [ 308.736047] [<c1075da9>] ? sched_clock_local+0x49/0x180 [ 308.736047] [<c109325d>] lock_acquire+0x9d/0x140 [ 308.736047] [<f82a8d4c>] ? hci_send_acl+0xac/0x290 [bluetooth] [ 308.736047] [<c1889c25>] _raw_spin_lock+0x45/0x80 [ 308.736047] [<f82a8d4c>] ? hci_send_acl+0xac/0x290 [bluetooth] [ 308.736047] [<f82a8d4c>] hci_send_acl+0xac/0x290 [bluetooth] [ 308.736047] [<c108f0b4>] ? mark_held_locks+0x64/0x90 [ 308.736047] [<c188a065>] ? _raw_spin_unlock_irqrestore+0x55/0x70 [ 308.736047] [<f82c9bf0>] l2cap_do_send+0x60/0x100 [bluetooth] [ 308.736047] [<c108f32b>] ? trace_hardirqs_on+0xb/0x10 [ 308.736047] [<c188a051>] ? _raw_spin_unlock_irqrestore+0x41/0x70 [ 308.736047] [<c1763125>] ? skb_dequeue+0x45/0x60 [ 308.736047] [<f82d4389>] l2cap_recv_frame+0x23d9/0x2db0 [bluetooth] [ 308.736047] [<c1075da9>] ? sched_clock_local+0x49/0x180 [ 308.736047] [<c100a5f8>] ? sched_clock+0x8/0x10 [ 308.736047] [<c1075da9>] ? sched_clock_local+0x49/0x180 [ 308.736047] [<c10761ef>] ? sched_clock_cpu+0x10f/0x160 [ 308.736047] [<c107183b>] ? get_parent_ip+0xb/0x40 [ 308.736047] [<c10718bb>] ? preempt_count_add+0x4b/0xa0 [ 308.736047] [<c13d09e2>] ? debug_smp_processor_id+0x12/0x20 [ 308.736047] [<c108cc04>] ? get_lock_stats+0x24/0x40 [ 308.736047] [<c108f0b4>] ? mark_held_locks+0x64/0x90 [ 308.736047] [<c188716d>] ? __mutex_unlock_slowpath+0xcd/0x1b0 [ 308.736047] [<c13d09ff>] ? __this_cpu_preempt_check+0xf/0x20 [ 308.736047] [<c108f1d4>] ? trace_hardirqs_on_caller+0xf4/0x240 [ 308.736047] [<c108c9a6>] ? trace_hardirqs_off_caller+0xb6/0x160 [ 308.736047] [<f82d62a9>] l2cap_recv_acldata+0x2f9/0x340 [bluetooth] [ 308.736047] [<f82a1a13>] ? hci_rx_work+0x113/0x4a0 [bluetooth] [ 308.736047] [<f82a1c69>] hci_rx_work+0x369/0x4a0 [bluetooth] [ 308.736047] [<f82a1a13>] ? hci_rx_work+0x113/0x4a0 [bluetooth] [ 308.736047] [<c106234a>] process_one_work+0x19a/0x4a0 [ 308.736047] [<c10622c3>] ? process_one_work+0x113/0x4a0 [ 308.736047] [<c10629e9>] worker_thread+0x39/0x440 [ 308.736047] [<c10629b0>] ? init_pwq+0xc0/0xc0 [ 308.736047] [<c1066dc8>] kthread+0xa8/0xc0 [ 308.736047] [<c108f32b>] ? trace_hardirqs_on+0xb/0x10 [ 308.736047] [<c188ad01>] ret_from_kernel_thread+0x21/0x30 [ 308.736047] [<c1066d20>] ? kthread_create_on_node+0x160/0x160 Cheers, Jukka ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 11:49 ` Jukka Rissanen @ 2014-10-13 13:10 ` Martin Townsend 2014-10-13 14:44 ` Jukka Rissanen 0 siblings, 1 reply; 16+ messages in thread From: Martin Townsend @ 2014-10-13 13:10 UTC (permalink / raw) To: Jukka Rissanen, Martin Townsend Cc: linux-bluetooth, linux-wpan, marcel, alex.aring Hi Jukka, I think there's a lock checking option in the kernel hacking configuration menu. Might be worth trying this to get more info. I had a quick look through the code and there maybe a potential locking problem in l2cap_le_credits it calls l2cap_get_chan_by_dcid which locks the channel lock (chan->lock) which is one of the locks in the deadlock below. If credits > max_credits in l2cap_le_credits it returns 0 but no unlock. Now l2cap_send_disconn_req may do this, I tried searching through but it called a state_change op so I gave up. http://lxr.free-electrons.com/source/net/bluetooth/l2cap_core.c#L5539 You could try sticking a l2cap_chan_unlock(chan); in to see if the problem goes away. Alex have you tried this patch in a virtual emulator or on real HW for 802.15.4? we would know if it's a bluetooth or general problem. - Martin On 13/10/14 12:49, Jukka Rissanen wrote: > Hi Martin, > > with this v6 I started to see similar issues locking issues as in some > earlier patch version. I will try v5 and v4 just to make sure what is > going on here. > > > [ 308.736047] ================================= > [ 308.736047] [ INFO: inconsistent lock state ] > [ 308.736047] 3.17.0-rc1-bt6lowpan #1 Not tainted > [ 308.736047] --------------------------------- > [ 308.736047] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage. > [ 308.736047] kworker/u3:2/404 [HC0[0]:SC0[0]:HE1:SE1] takes: > [ 308.736047] (&(&list->lock)->rlock#6){+.?...}, at: [<f82a8d4c>] > hci_send_acl+0xac/0x290 [bluetooth] > [ 308.736047] {IN-SOFTIRQ-W} state was registered at: > [ 308.736047] [<c10915a3>] __lock_acquire+0x6d3/0x1d20 > [ 308.736047] [<c109325d>] lock_acquire+0x9d/0x140 > [ 308.736047] [<c1889c25>] _raw_spin_lock+0x45/0x80 > [ 308.736047] [<f82a8d4c>] hci_send_acl+0xac/0x290 [bluetooth] > [ 308.736047] [<f82c9bf0>] l2cap_do_send+0x60/0x100 [bluetooth] > [ 308.736047] [<f82cd7c0>] l2cap_chan_send+0x7f0/0x10e0 [bluetooth] > [ 308.736047] [<f850d91e>] send_pkt+0x4e/0xa0 [bluetooth_6lowpan] > [ 308.736047] [<f850dd20>] bt_xmit+0x3b0/0x770 [bluetooth_6lowpan] > [ 308.736047] [<c17742f4>] dev_hard_start_xmit+0x344/0x670 > [ 308.736047] [<c17749ad>] __dev_queue_xmit+0x38d/0x680 > [ 308.736047] [<c1774caf>] dev_queue_xmit+0xf/0x20 > [ 308.736047] [<c177b8b0>] neigh_connected_output+0x130/0x1a0 > [ 308.736047] [<c1812a63>] ip6_finish_output2+0x173/0x8c0 > [ 308.736047] [<c18182db>] ip6_finish_output+0x7b/0x1b0 > [ 308.736047] [<c18184a7>] ip6_output+0x97/0x2a0 > [ 308.736047] [<c183a46b>] mld_sendpack+0x5eb/0x650 > [ 308.736047] [<c183acc1>] mld_ifc_timer_expire+0x191/0x2f0 > [ 308.736047] [<c10ac385>] call_timer_fn+0x85/0x1c0 > [ 308.736047] [<c10acb72>] run_timer_softirq+0x192/0x280 > [ 308.736047] [<c104fd84>] __do_softirq+0xd4/0x300 > [ 308.736047] [<c10049fc>] do_softirq_own_stack+0x2c/0x40 > [ 308.736047] [<c1050136>] irq_exit+0x86/0xb0 > [ 308.736047] [<c188bd98>] smp_apic_timer_interrupt+0x38/0x50 > [ 308.736047] [<c188b6ce>] apic_timer_interrupt+0x32/0x38 > [ 308.736047] [<c115e6b2>] kmem_cache_alloc+0x1c2/0x290 > [ 308.736047] [<c116a9ae>] create_object+0x2e/0x280 > [ 308.736047] [<c187f10c>] kmemleak_alloc+0x3c/0xb0 > [ 308.736047] [<c115e693>] kmem_cache_alloc+0x1a3/0x290 > [ 308.736047] [<c1178835>] getname_flags+0x25/0x110 > [ 308.736047] [<c117d21e>] user_path_at_empty+0x1e/0x80 > [ 308.736047] [<c1173317>] SyS_readlinkat+0x57/0x100 > [ 308.736047] [<c11733ec>] SyS_readlink+0x2c/0x30 > [ 308.736047] [<c188aeb6>] syscall_after_call+0x0/0x4 > [ 308.736047] irq event stamp: 37665 > [ 308.736047] hardirqs last enabled at (37665): [<c188a065>] > _raw_spin_unlock_irqrestore+0x55/0x70 > [ 308.736047] hardirqs last disabled at (37664): [<c1889e03>] > _raw_spin_lock_irqsave+0x23/0x90 > [ 308.736047] softirqs last enabled at (37378): [<c104ff5c>] > __do_softirq+0x2ac/0x300 > [ 308.736047] softirqs last disabled at (37359): [<c10049fc>] > do_softirq_own_stack+0x2c/0x40 > [ 308.736047] > [ 308.736047] other info that might help us debug this: > [ 308.736047] Possible unsafe locking scenario: > [ 308.736047] > [ 308.736047] CPU0 > [ 308.736047] ---- > [ 308.736047] lock(&(&list->lock)->rlock#6); > [ 308.736047] <Interrupt> > [ 308.736047] lock(&(&list->lock)->rlock#6); > [ 308.736047] > [ 308.736047] *** DEADLOCK *** > [ 308.736047] > [ 308.736047] 3 locks held by kworker/u3:2/404: > [ 308.736047] #0: ("%s"hdev->name#2){.+.+.+}, at: [<c10622c3>] > process_one_work+0x113/0x4a0 > [ 308.736047] #1: ((&hdev->rx_work)){+.+.+.}, at: [<c10622c3>] > process_one_work+0x113/0x4a0 > [ 308.736047] #2: (&chan->lock){+.+.+.}, at: [<f82c9179>] > l2cap_get_chan_by_dcid+0x89/0x90 [bluetooth] > [ 308.736047] > [ 308.736047] stack backtrace: > [ 308.736047] CPU: 0 PID: 404 Comm: kworker/u3:2 Not tainted > 3.17.0-rc1-bt6lowpan #1 > [ 308.736047] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS > VirtualBox 12/01/2006 > [ 308.736047] Workqueue: hci0 hci_rx_work [bluetooth] > [ 308.736047] efe3d780 00000000 efedbb90 c18821c1 c2345c10 efedbbb0 > c1880a94 c1ad9264 > [ 308.736047] c1ad9201 c1ad95f8 efe3dd94 00000006 c108e0c0 efedbbe0 > c108ea8c 00000006 > [ 308.736047] efe3d780 efedbffc e21128f4 00000047 00000004 efe3d780 > efe3dd98 00000003 > [ 308.736047] Call Trace: > [ 308.736047] [<c18821c1>] dump_stack+0x4b/0x75 > [ 308.736047] [<c1880a94>] print_usage_bug.part.36+0x209/0x213 > [ 308.736047] [<c108e0c0>] ? check_usage_forwards+0x110/0x110 > [ 308.736047] [<c108ea8c>] mark_lock+0x11c/0x6e0 > [ 308.736047] [<c10915e4>] __lock_acquire+0x714/0x1d20 > [ 308.736047] [<c1004b6f>] ? dump_trace+0xcf/0x1f0 > [ 308.736047] [<c100a5f8>] ? sched_clock+0x8/0x10 > [ 308.736047] [<c1075da9>] ? sched_clock_local+0x49/0x180 > [ 308.736047] [<c109325d>] lock_acquire+0x9d/0x140 > [ 308.736047] [<f82a8d4c>] ? hci_send_acl+0xac/0x290 [bluetooth] > [ 308.736047] [<c1889c25>] _raw_spin_lock+0x45/0x80 > [ 308.736047] [<f82a8d4c>] ? hci_send_acl+0xac/0x290 [bluetooth] > [ 308.736047] [<f82a8d4c>] hci_send_acl+0xac/0x290 [bluetooth] > [ 308.736047] [<c108f0b4>] ? mark_held_locks+0x64/0x90 > [ 308.736047] [<c188a065>] ? _raw_spin_unlock_irqrestore+0x55/0x70 > [ 308.736047] [<f82c9bf0>] l2cap_do_send+0x60/0x100 [bluetooth] > [ 308.736047] [<c108f32b>] ? trace_hardirqs_on+0xb/0x10 > [ 308.736047] [<c188a051>] ? _raw_spin_unlock_irqrestore+0x41/0x70 > [ 308.736047] [<c1763125>] ? skb_dequeue+0x45/0x60 > [ 308.736047] [<f82d4389>] l2cap_recv_frame+0x23d9/0x2db0 [bluetooth] > [ 308.736047] [<c1075da9>] ? sched_clock_local+0x49/0x180 > [ 308.736047] [<c100a5f8>] ? sched_clock+0x8/0x10 > [ 308.736047] [<c1075da9>] ? sched_clock_local+0x49/0x180 > [ 308.736047] [<c10761ef>] ? sched_clock_cpu+0x10f/0x160 > [ 308.736047] [<c107183b>] ? get_parent_ip+0xb/0x40 > [ 308.736047] [<c10718bb>] ? preempt_count_add+0x4b/0xa0 > [ 308.736047] [<c13d09e2>] ? debug_smp_processor_id+0x12/0x20 > [ 308.736047] [<c108cc04>] ? get_lock_stats+0x24/0x40 > [ 308.736047] [<c108f0b4>] ? mark_held_locks+0x64/0x90 > [ 308.736047] [<c188716d>] ? __mutex_unlock_slowpath+0xcd/0x1b0 > [ 308.736047] [<c13d09ff>] ? __this_cpu_preempt_check+0xf/0x20 > [ 308.736047] [<c108f1d4>] ? trace_hardirqs_on_caller+0xf4/0x240 > [ 308.736047] [<c108c9a6>] ? trace_hardirqs_off_caller+0xb6/0x160 > [ 308.736047] [<f82d62a9>] l2cap_recv_acldata+0x2f9/0x340 [bluetooth] > [ 308.736047] [<f82a1a13>] ? hci_rx_work+0x113/0x4a0 [bluetooth] > [ 308.736047] [<f82a1c69>] hci_rx_work+0x369/0x4a0 [bluetooth] > [ 308.736047] [<f82a1a13>] ? hci_rx_work+0x113/0x4a0 [bluetooth] > [ 308.736047] [<c106234a>] process_one_work+0x19a/0x4a0 > [ 308.736047] [<c10622c3>] ? process_one_work+0x113/0x4a0 > [ 308.736047] [<c10629e9>] worker_thread+0x39/0x440 > [ 308.736047] [<c10629b0>] ? init_pwq+0xc0/0xc0 > [ 308.736047] [<c1066dc8>] kthread+0xa8/0xc0 > [ 308.736047] [<c108f32b>] ? trace_hardirqs_on+0xb/0x10 > [ 308.736047] [<c188ad01>] ret_from_kernel_thread+0x21/0x30 > [ 308.736047] [<c1066d20>] ? kthread_create_on_node+0x160/0x160 > > > > Cheers, > Jukka > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 13:10 ` Martin Townsend @ 2014-10-13 14:44 ` Jukka Rissanen 2014-10-13 14:56 ` Martin Townsend 0 siblings, 1 reply; 16+ messages in thread From: Jukka Rissanen @ 2014-10-13 14:44 UTC (permalink / raw) To: Martin Townsend Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring Hi Martin, and thanks for your analysis. On ma, 2014-10-13 at 14:10 +0100, Martin Townsend wrote: > Hi Jukka, > > I think there's a lock checking option in the kernel hacking configuration menu. Might be worth trying this to get more info. > I had a quick look through the code and there maybe a potential locking problem in l2cap_le_credits > it calls l2cap_get_chan_by_dcid which locks the channel lock (chan->lock) which is one of the locks in the deadlock below. If credits > max_credits in l2cap_le_credits it returns 0 but no unlock. Now l2cap_send_disconn_req may do this, I tried searching through but it called a state_change op so I gave up. > http://lxr.free-electrons.com/source/net/bluetooth/l2cap_core.c#L5539 > > You could try sticking a l2cap_chan_unlock(chan); in to see if the problem goes away. > I managed to trigger the locking issue (by running ssh over bt 6lowpan) even without your patch. So I am acking the v6 of this patch. I try to dig the root cause to that deadlock issue I am seeing. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com> Cheers, Jukka ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 14:44 ` Jukka Rissanen @ 2014-10-13 14:56 ` Martin Townsend 2014-10-13 15:09 ` Jukka Rissanen 0 siblings, 1 reply; 16+ messages in thread From: Martin Townsend @ 2014-10-13 14:56 UTC (permalink / raw) To: Jukka Rissanen Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring Hi Jukka, Does this patch help? --- net/bluetooth/l2cap_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index b6f9777..fb7b2ff 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn, if (credits > max_credits) { BT_ERR("LE credits overflow"); l2cap_send_disconn_req(chan, ECONNRESET); + l2cap_chan_unlock(chan); /* Return 0 so that we don't trigger an unnecessary * command reject packet. -- 1.9.1 -Martin. On 13/10/14 15:44, Jukka Rissanen wrote: > Hi Martin, > > and thanks for your analysis. > > On ma, 2014-10-13 at 14:10 +0100, Martin Townsend wrote: >> Hi Jukka, >> >> I think there's a lock checking option in the kernel hacking configuration menu. Might be worth trying this to get more info. >> I had a quick look through the code and there maybe a potential locking problem in l2cap_le_credits >> it calls l2cap_get_chan_by_dcid which locks the channel lock (chan->lock) which is one of the locks in the deadlock below. If credits > max_credits in l2cap_le_credits it returns 0 but no unlock. Now l2cap_send_disconn_req may do this, I tried searching through but it called a state_change op so I gave up. >> http://lxr.free-electrons.com/source/net/bluetooth/l2cap_core.c#L5539 >> >> You could try sticking a l2cap_chan_unlock(chan); in to see if the problem goes away. >> > I managed to trigger the locking issue (by running ssh over bt 6lowpan) > even without your patch. So I am acking the v6 of this patch. I try to > dig the root cause to that deadlock issue I am seeing. > > Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com> > > > Cheers, > Jukka > > ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 14:56 ` Martin Townsend @ 2014-10-13 15:09 ` Jukka Rissanen 2014-10-13 15:47 ` Martin Townsend ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: Jukka Rissanen @ 2014-10-13 15:09 UTC (permalink / raw) To: Martin Townsend Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring Hi Martin, On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote: > Hi Jukka, > > Does this patch help? Unfortunately no, I still see inconsistent lock state. It would probably have been too easy :) > > --- > net/bluetooth/l2cap_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index b6f9777..fb7b2ff 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn, > if (credits > max_credits) { > BT_ERR("LE credits overflow"); > l2cap_send_disconn_req(chan, ECONNRESET); > + l2cap_chan_unlock(chan); > > /* Return 0 so that we don't trigger an unnecessary > * command reject packet. Cheers, Jukka ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 15:09 ` Jukka Rissanen @ 2014-10-13 15:47 ` Martin Townsend 2014-10-13 16:00 ` Martin Townsend ` (2 subsequent siblings) 3 siblings, 0 replies; 16+ messages in thread From: Martin Townsend @ 2014-10-13 15:47 UTC (permalink / raw) To: Jukka Rissanen Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring [-- Attachment #1: Type: text/plain, Size: 2024 bytes --] Hi Jukka, >From the last trace it looks like a transmit has been started from the receive worker thread. I notice that both tx and rx use the same workerqueue structure, e.g. hci_send_xxx <http://lxr.free-electrons.com/ident?i=hci_send_acl> ... queue_work <http://lxr.free-electrons.com/ident?i=queue_work>(hdev <http://lxr.free-electrons.com/ident?i=hdev>->workqueue <http://lxr.free-electrons.com/ident?i=workqueue>, &hdev <http://lxr.free-electrons.com/ident?i=hdev>->tx_work); hci_recv_frame <http://lxr.free-electrons.com/ident?i=hci_recv_frame> ... queue_work <http://lxr.free-electrons.com/ident?i=queue_work>(hdev <http://lxr.free-electrons.com/ident?i=hdev>->workqueue <http://lxr.free-electrons.com/ident?i=workqueue>, &hdev <http://lxr.free-electrons.com/ident?i=hdev>->rx_work <http://lxr.free-electrons.com/ident?i=rx_work>); Would this cause problems. I don't know enough about work queues but in workqueue_struct there is a mutex which may be held by the rx worker and if this rx worker start a transmit maybe this would cause the lock? Could test by creating separate queues for rx and tx? Anyway just a thought. - Martin. On 13/10/14 16:09, Jukka Rissanen wrote: > Hi Martin, > > On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote: >> Hi Jukka, >> >> Does this patch help? > Unfortunately no, I still see inconsistent lock state. It would probably > have been too easy :) > >> --- >> net/bluetooth/l2cap_core.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >> index b6f9777..fb7b2ff 100644 >> --- a/net/bluetooth/l2cap_core.c >> +++ b/net/bluetooth/l2cap_core.c >> @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn, >> if (credits > max_credits) { >> BT_ERR("LE credits overflow"); >> l2cap_send_disconn_req(chan, ECONNRESET); >> + l2cap_chan_unlock(chan); >> >> /* Return 0 so that we don't trigger an unnecessary >> * command reject packet. > > Cheers, > Jukka > > [-- Attachment #2: Type: text/html, Size: 2931 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 15:09 ` Jukka Rissanen 2014-10-13 15:47 ` Martin Townsend @ 2014-10-13 16:00 ` Martin Townsend 2014-10-13 16:07 ` Johan Hedberg 2014-10-13 17:11 ` Alexander Aring 3 siblings, 0 replies; 16+ messages in thread From: Martin Townsend @ 2014-10-13 16:00 UTC (permalink / raw) To: Jukka Rissanen Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring Hi Jukka, >From the last trace it looks like a transmit has been started from the receive worker thread. I notice that both tx and rx use the same workerqueue structure, e.g. hci_send_xxx ... queue_work(hdev->workqueue, &hdev->tx_work); hci_recv_frame ... queue_work(hdev->workqueue, &hdev->rx_work); Would this cause problems. I don't know enough about work queues but in workqueue_struct there is a mutex which may be held by the rx worker and if this rx worker start a transmit maybe this would cause the lock? Could test by creating separate queues for rx and tx? Anyway just a thought. - Martin. On 13/10/14 16:09, Jukka Rissanen wrote: > Hi Martin, > > On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote: >> Hi Jukka, >> >> Does this patch help? > Unfortunately no, I still see inconsistent lock state. It would probably > have been too easy :) > >> --- >> net/bluetooth/l2cap_core.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >> index b6f9777..fb7b2ff 100644 >> --- a/net/bluetooth/l2cap_core.c >> +++ b/net/bluetooth/l2cap_core.c >> @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn, >> if (credits > max_credits) { >> BT_ERR("LE credits overflow"); >> l2cap_send_disconn_req(chan, ECONNRESET); >> + l2cap_chan_unlock(chan); >> >> /* Return 0 so that we don't trigger an unnecessary >> * command reject packet. > > Cheers, > Jukka > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 15:09 ` Jukka Rissanen 2014-10-13 15:47 ` Martin Townsend 2014-10-13 16:00 ` Martin Townsend @ 2014-10-13 16:07 ` Johan Hedberg 2014-10-13 17:11 ` Alexander Aring 3 siblings, 0 replies; 16+ messages in thread From: Johan Hedberg @ 2014-10-13 16:07 UTC (permalink / raw) To: Jukka Rissanen Cc: Martin Townsend, Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring Hi, On Mon, Oct 13, 2014, Jukka Rissanen wrote: > On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote: > > Hi Jukka, > > > > Does this patch help? > > Unfortunately no, I still see inconsistent lock state. It would probably > have been too easy :) > > > > --- > > net/bluetooth/l2cap_core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index b6f9777..fb7b2ff 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn, > > if (credits > max_credits) { > > BT_ERR("LE credits overflow"); > > l2cap_send_disconn_req(chan, ECONNRESET); > > + l2cap_chan_unlock(chan); > > > > /* Return 0 so that we don't trigger an unnecessary > > * command reject packet. I'd appreciate if you could still send a proper patch for this since it's clearly a locking bug (something even worth sending to stable trees). Johan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 15:09 ` Jukka Rissanen ` (2 preceding siblings ...) 2014-10-13 16:07 ` Johan Hedberg @ 2014-10-13 17:11 ` Alexander Aring 2014-10-13 17:22 ` Alexander Aring 3 siblings, 1 reply; 16+ messages in thread From: Alexander Aring @ 2014-10-13 17:11 UTC (permalink / raw) To: Jukka Rissanen Cc: Martin Townsend, Martin Townsend, linux-bluetooth, linux-wpan, marcel Hi Jukka, On Mon, Oct 13, 2014 at 06:09:14PM +0300, Jukka Rissanen wrote: > Hi Martin, > > On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote: > > Hi Jukka, > > > > Does this patch help? > > Unfortunately no, I still see inconsistent lock state. It would probably > have been too easy :) > I remeber something, I think 802.15.4 had similar issues long time ago. The fix was 20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 ("6lowpan: fix lockdep splats"). Please check that, you need something like that! - Alex [0] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 17:11 ` Alexander Aring @ 2014-10-13 17:22 ` Alexander Aring 2014-10-14 8:35 ` Jukka Rissanen 0 siblings, 1 reply; 16+ messages in thread From: Alexander Aring @ 2014-10-13 17:22 UTC (permalink / raw) To: Jukka Rissanen Cc: Martin Townsend, Martin Townsend, linux-bluetooth, linux-wpan, marcel Hi Jukka, sorry. I was a little too fast here, because I am sure now this should solve your lockdep issue. On Mon, Oct 13, 2014 at 07:11:11PM +0200, Alexander Aring wrote: > Hi Jukka, > > On Mon, Oct 13, 2014 at 06:09:14PM +0300, Jukka Rissanen wrote: > > Hi Martin, > > > > On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote: > > > Hi Jukka, > > > > > > Does this patch help? > > > > Unfortunately no, I still see inconsistent lock state. It would probably > > have been too easy :) > > > > I remeber something, I think 802.15.4 had similar issues long time ago. > s/remeber/remember/ > The fix was 20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 ("6lowpan: fix > lockdep splats"). Please check that, you need something like that! > Something like that: diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index 4ebc806..02fd21a 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -642,7 +642,27 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev) return err < 0 ? NET_XMIT_DROP : err; } +static struct lock_class_key bt_tx_busylock; +static struct lock_class_key bt_netdev_xmit_lock_key; + +static void bt_set_lockdep_class_one(struct net_device *dev, + struct netdev_queue *txq, + void *_unused) +{ + lockdep_set_class(&txq->_xmit_lock, + &bt_netdev_xmit_lock_key); +} + + +static int bt_dev_init(struct net_device *dev) +{ + netdev_for_each_tx_queue(dev, bt_set_lockdep_class_one, NULL); + dev->qdisc_tx_busylock = &bt_tx_busylock; + return 0; +} + static const struct net_device_ops netdev_ops = { + .ndo_init = bt_dev_init, .ndo_start_xmit = bt_xmit, }; - Alex [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 17:22 ` Alexander Aring @ 2014-10-14 8:35 ` Jukka Rissanen 2014-10-14 8:38 ` Alexander Aring 0 siblings, 1 reply; 16+ messages in thread From: Jukka Rissanen @ 2014-10-14 8:35 UTC (permalink / raw) To: Alexander Aring Cc: Martin Townsend, Martin Townsend, linux-bluetooth, linux-wpan, marcel Hi Alex, On ma, 2014-10-13 at 19:22 +0200, Alexander Aring wrote: > Hi Jukka, > > sorry. > > I was a little too fast here, because I am sure now this should solve your > lockdep issue. Unfortunately this patch did not help with the issue. > > On Mon, Oct 13, 2014 at 07:11:11PM +0200, Alexander Aring wrote: > > Hi Jukka, > > > > On Mon, Oct 13, 2014 at 06:09:14PM +0300, Jukka Rissanen wrote: > > > Hi Martin, > > > > > > On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote: > > > > Hi Jukka, > > > > > > > > Does this patch help? > > > > > > Unfortunately no, I still see inconsistent lock state. It would probably > > > have been too easy :) > > > > > > > I remeber something, I think 802.15.4 had similar issues long time ago. > > > s/remeber/remember/ > > > The fix was 20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 ("6lowpan: fix > > lockdep splats"). Please check that, you need something like that! > > > > Something like that: > > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c > index 4ebc806..02fd21a 100644 > --- a/net/bluetooth/6lowpan.c > +++ b/net/bluetooth/6lowpan.c > @@ -642,7 +642,27 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev) > return err < 0 ? NET_XMIT_DROP : err; > } > > +static struct lock_class_key bt_tx_busylock; > +static struct lock_class_key bt_netdev_xmit_lock_key; > + > +static void bt_set_lockdep_class_one(struct net_device *dev, > + struct netdev_queue *txq, > + void *_unused) > +{ > + lockdep_set_class(&txq->_xmit_lock, > + &bt_netdev_xmit_lock_key); > +} > + > + > +static int bt_dev_init(struct net_device *dev) > +{ > + netdev_for_each_tx_queue(dev, bt_set_lockdep_class_one, NULL); > + dev->qdisc_tx_busylock = &bt_tx_busylock; > + return 0; > +} > + > static const struct net_device_ops netdev_ops = { > + .ndo_init = bt_dev_init, > .ndo_start_xmit = bt_xmit, > }; > > > - Alex > > [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 Martin had an idea about the issue being related to work queues, I will investigate that further. Cheers, Jukka ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-14 8:35 ` Jukka Rissanen @ 2014-10-14 8:38 ` Alexander Aring 0 siblings, 0 replies; 16+ messages in thread From: Alexander Aring @ 2014-10-14 8:38 UTC (permalink / raw) To: Jukka Rissanen Cc: Martin Townsend, Martin Townsend, linux-bluetooth, linux-wpan, marcel Hi Jukka, On Tue, Oct 14, 2014 at 11:35:05AM +0300, Jukka Rissanen wrote: > Hi Alex, > > On ma, 2014-10-13 at 19:22 +0200, Alexander Aring wrote: > > Hi Jukka, > > > > sorry. > > > > I was a little too fast here, because I am sure now this should solve your > > lockdep issue. > > Unfortunately this patch did not help with the issue. > mhh, ok. > > > > On Mon, Oct 13, 2014 at 07:11:11PM +0200, Alexander Aring wrote: > > > Hi Jukka, > > > > > > On Mon, Oct 13, 2014 at 06:09:14PM +0300, Jukka Rissanen wrote: > > > > Hi Martin, > > > > > > > > On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote: > > > > > Hi Jukka, > > > > > > > > > > Does this patch help? > > > > > > > > Unfortunately no, I still see inconsistent lock state. It would probably > > > > have been too easy :) > > > > > > > > > > I remeber something, I think 802.15.4 had similar issues long time ago. > > > > > s/remeber/remember/ > > > > > The fix was 20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 ("6lowpan: fix > > > lockdep splats"). Please check that, you need something like that! > > > > > > > Something like that: > > > > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c > > index 4ebc806..02fd21a 100644 > > --- a/net/bluetooth/6lowpan.c > > +++ b/net/bluetooth/6lowpan.c > > @@ -642,7 +642,27 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev) > > return err < 0 ? NET_XMIT_DROP : err; > > } > > > > +static struct lock_class_key bt_tx_busylock; > > +static struct lock_class_key bt_netdev_xmit_lock_key; > > + > > +static void bt_set_lockdep_class_one(struct net_device *dev, > > + struct netdev_queue *txq, > > + void *_unused) > > +{ > > + lockdep_set_class(&txq->_xmit_lock, > > + &bt_netdev_xmit_lock_key); > > +} > > + > > + > > +static int bt_dev_init(struct net_device *dev) > > +{ > > + netdev_for_each_tx_queue(dev, bt_set_lockdep_class_one, NULL); > > + dev->qdisc_tx_busylock = &bt_tx_busylock; > > + return 0; > > +} > > + > > static const struct net_device_ops netdev_ops = { > > + .ndo_init = bt_dev_init, > > .ndo_start_xmit = bt_xmit, > > }; > > > > > > - Alex > > > > [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 > > > Martin had an idea about the issue being related to work queues, I will > investigate that further. > ok. The above one should fix if some dev_queue_xmit call another dev_queue_xmit. Don't know if you have some architecture like this in bluetooth. - Alex ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression. 2014-10-13 10:00 ` Martin Townsend 2014-10-13 10:34 ` Alexander Aring 2014-10-13 11:49 ` Jukka Rissanen @ 2014-10-17 14:15 ` Marcel Holtmann 2 siblings, 0 replies; 16+ messages in thread From: Marcel Holtmann @ 2014-10-17 14:15 UTC (permalink / raw) To: Martin Townsend Cc: linux-bluetooth, linux-wpan, alex.aring, jukka.rissanen, Martin Townsend Hi Martin, > Currently there are potentially 2 skb_copy_expand calls in IPHC > decompression. This patch replaces this with one call to > skb_cow which will check to see if there is enough headroom > first to ensure it's only done if necessary and will handle > alignment issues for cache. > As skb_cow uses pskb_expand_head we ensure the skb isn't shared from > bluetooth and ieee802.15.4 code that use the IPHC decompression. > > Signed-off-by: Martin Townsend <martin.townsend@xsilon.com> > --- > net/6lowpan/iphc.c | 47 +++++++++++++++++++++-------------------------- > net/bluetooth/6lowpan.c | 4 ++++ > 2 files changed, 25 insertions(+), 26 deletions(-) patch has been applied to bluetooth-next tree. Regards Marcel ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-10-17 14:15 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-13 10:00 [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression Martin Townsend 2014-10-13 10:00 ` Martin Townsend 2014-10-13 10:34 ` Alexander Aring 2014-10-13 11:49 ` Jukka Rissanen 2014-10-13 13:10 ` Martin Townsend 2014-10-13 14:44 ` Jukka Rissanen 2014-10-13 14:56 ` Martin Townsend 2014-10-13 15:09 ` Jukka Rissanen 2014-10-13 15:47 ` Martin Townsend 2014-10-13 16:00 ` Martin Townsend 2014-10-13 16:07 ` Johan Hedberg 2014-10-13 17:11 ` Alexander Aring 2014-10-13 17:22 ` Alexander Aring 2014-10-14 8:35 ` Jukka Rissanen 2014-10-14 8:38 ` Alexander Aring 2014-10-17 14:15 ` Marcel Holtmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).