2012-07-13 07:52:03

by Jaganath Kanakkassery

[permalink] [raw]
Subject: [PATCH] Bluetooth: Free the l2cap channel list only when refcount is zero

Move the l2cap channel list chan->global_l under the refcnt
protection and free it based on the refcnt.

Signed-off-by: Syam Sidhardhan <[email protected]>
Signed-off-by: Jaganath Kanakkassery <[email protected]>
---
include/net/bluetooth/l2cap.h | 5 +++--
net/bluetooth/a2mp.c | 2 +-
net/bluetooth/l2cap_core.c | 4 +++-
net/bluetooth/l2cap_sock.c | 2 +-
4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index e5164ff..4c0dcba 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -670,6 +670,8 @@ enum {
L2CAP_EV_RECV_FRAME,
};

+void l2cap_chan_destroy(struct l2cap_chan *chan);
+
static inline void l2cap_chan_hold(struct l2cap_chan *c)
{
BT_DBG("chan %p orig refcnt %d", c, atomic_read(&c->refcnt));
@@ -682,7 +684,7 @@ static inline void l2cap_chan_put(struct l2cap_chan *c)
BT_DBG("chan %p orig refcnt %d", c, atomic_read(&c->refcnt));

if (atomic_dec_and_test(&c->refcnt))
- kfree(c);
+ l2cap_chan_destroy(c);
}

static inline void l2cap_chan_lock(struct l2cap_chan *chan)
@@ -770,7 +772,6 @@ int l2cap_add_scid(struct l2cap_chan *chan, __u16 scid);

struct l2cap_chan *l2cap_chan_create(void);
void l2cap_chan_close(struct l2cap_chan *chan, int reason);
-void l2cap_chan_destroy(struct l2cap_chan *chan);
int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
bdaddr_t *dst, u8 dst_type);
int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index 4ff0bf3..cf16a7c 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -412,7 +412,7 @@ static int a2mp_chan_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)

static void a2mp_chan_close_cb(struct l2cap_chan *chan)
{
- l2cap_chan_destroy(chan);
+ l2cap_chan_put(chan);
}

static void a2mp_chan_state_change_cb(struct l2cap_chan *chan, int state)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9fd0599..8718a22 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -418,11 +418,13 @@ struct l2cap_chan *l2cap_chan_create(void)

void l2cap_chan_destroy(struct l2cap_chan *chan)
{
+ BT_DBG("chan %p", chan);
+
write_lock(&chan_list_lock);
list_del(&chan->global_l);
write_unlock(&chan_list_lock);

- l2cap_chan_put(chan);
+ kfree(chan);
}

void l2cap_chan_set_defaults(struct l2cap_chan *chan)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index a4bb27e..79350d1 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -823,7 +823,7 @@ static void l2cap_sock_kill(struct sock *sk)

/* Kill poor orphan */

- l2cap_chan_destroy(l2cap_pi(sk)->chan);
+ l2cap_chan_put(l2cap_pi(sk)->chan);
sock_set_flag(sk, SOCK_DEAD);
sock_put(sk);
}
--
1.7.4.1



2012-07-13 11:26:02

by Jaganath Kanakkassery

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when refcount is zero

Hi Andrei,

--------------------------------------------------
From: "Andrei Emeltchenko" <[email protected]>
Sent: Friday, July 13, 2012 2:19 PM
To: "Jaganath Kanakkassery" <[email protected]>
Cc: <[email protected]>; <[email protected]>
Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when
refcount is zero

> Hi Jaganath,
>
> On Fri, Jul 13, 2012 at 01:22:03PM +0530, Jaganath Kanakkassery wrote:
>> Move the l2cap channel list chan->global_l under the refcnt
>> protection and free it based on the refcnt.
>
> The idea is good.
>
>> Signed-off-by: Syam Sidhardhan <[email protected]>
>> Signed-off-by: Jaganath Kanakkassery <[email protected]>
>> ---
>> include/net/bluetooth/l2cap.h | 5 +++--
>> net/bluetooth/a2mp.c | 2 +-
>> net/bluetooth/l2cap_core.c | 4 +++-
>> net/bluetooth/l2cap_sock.c | 2 +-
>> 4 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h
>> b/include/net/bluetooth/l2cap.h
>> index e5164ff..4c0dcba 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -670,6 +670,8 @@ enum {
>> L2CAP_EV_RECV_FRAME,
>> };
>>
>> +void l2cap_chan_destroy(struct l2cap_chan *chan);
>> +
>
> What I think is that l2cap_chan_destroy shall be static if it is used only
> inside l2cap_chan_put. Then l2cap_chan_hold/put shall be moved to
> l2cap_core or l2cap_chan_destroy moved to header. First is better IMO.

Ok. We will split the patches into two.
1. Refactoring patch which moves l2cap_chan_put() and l2cap_chan_get()
to l2cap_core.c and the

2. Moving the l2cap channel list chan->global_l under the refcnt protection

>> static inline void l2cap_chan_hold(struct l2cap_chan *c)
>> {
>> BT_DBG("chan %p orig refcnt %d", c, atomic_read(&c->refcnt));
>> @@ -682,7 +684,7 @@ static inline void l2cap_chan_put(struct l2cap_chan
>> *c)
>> BT_DBG("chan %p orig refcnt %d", c, atomic_read(&c->refcnt));
>>
>> if (atomic_dec_and_test(&c->refcnt))
>> - kfree(c);
>> + l2cap_chan_destroy(c);
>
> It does make sense to use kref mechanism here.

Ok. Once this patch is done we will raise the kref patch.

> Best regards
> Andrei Emeltchenko

Thanks,
Jaganath




2012-07-13 08:49:01

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when refcount is zero

Hi Jaganath,

On Fri, Jul 13, 2012 at 01:22:03PM +0530, Jaganath Kanakkassery wrote:
> Move the l2cap channel list chan->global_l under the refcnt
> protection and free it based on the refcnt.

The idea is good.

> Signed-off-by: Syam Sidhardhan <[email protected]>
> Signed-off-by: Jaganath Kanakkassery <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 5 +++--
> net/bluetooth/a2mp.c | 2 +-
> net/bluetooth/l2cap_core.c | 4 +++-
> net/bluetooth/l2cap_sock.c | 2 +-
> 4 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index e5164ff..4c0dcba 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -670,6 +670,8 @@ enum {
> L2CAP_EV_RECV_FRAME,
> };
>
> +void l2cap_chan_destroy(struct l2cap_chan *chan);
> +

What I think is that l2cap_chan_destroy shall be static if it is used only
inside l2cap_chan_put. Then l2cap_chan_hold/put shall be moved to
l2cap_core or l2cap_chan_destroy moved to header. First is better IMO.

> static inline void l2cap_chan_hold(struct l2cap_chan *c)
> {
> BT_DBG("chan %p orig refcnt %d", c, atomic_read(&c->refcnt));
> @@ -682,7 +684,7 @@ static inline void l2cap_chan_put(struct l2cap_chan *c)
> BT_DBG("chan %p orig refcnt %d", c, atomic_read(&c->refcnt));
>
> if (atomic_dec_and_test(&c->refcnt))
> - kfree(c);
> + l2cap_chan_destroy(c);

It does make sense to use kref mechanism here.

Best regards
Andrei Emeltchenko

> }
>
> static inline void l2cap_chan_lock(struct l2cap_chan *chan)
> @@ -770,7 +772,6 @@ int l2cap_add_scid(struct l2cap_chan *chan, __u16 scid);
>
> struct l2cap_chan *l2cap_chan_create(void);
> void l2cap_chan_close(struct l2cap_chan *chan, int reason);
> -void l2cap_chan_destroy(struct l2cap_chan *chan);
> int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> bdaddr_t *dst, u8 dst_type);
> int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> index 4ff0bf3..cf16a7c 100644
> --- a/net/bluetooth/a2mp.c
> +++ b/net/bluetooth/a2mp.c
> @@ -412,7 +412,7 @@ static int a2mp_chan_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
>
> static void a2mp_chan_close_cb(struct l2cap_chan *chan)
> {
> - l2cap_chan_destroy(chan);
> + l2cap_chan_put(chan);
> }
>
> static void a2mp_chan_state_change_cb(struct l2cap_chan *chan, int state)
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 9fd0599..8718a22 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -418,11 +418,13 @@ struct l2cap_chan *l2cap_chan_create(void)
>
> void l2cap_chan_destroy(struct l2cap_chan *chan)
> {
> + BT_DBG("chan %p", chan);
> +
> write_lock(&chan_list_lock);
> list_del(&chan->global_l);
> write_unlock(&chan_list_lock);
>
> - l2cap_chan_put(chan);
> + kfree(chan);
> }
>
> void l2cap_chan_set_defaults(struct l2cap_chan *chan)
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index a4bb27e..79350d1 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -823,7 +823,7 @@ static void l2cap_sock_kill(struct sock *sk)
>
> /* Kill poor orphan */
>
> - l2cap_chan_destroy(l2cap_pi(sk)->chan);
> + l2cap_chan_put(l2cap_pi(sk)->chan);
> sock_set_flag(sk, SOCK_DEAD);
> sock_put(sk);
> }
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2012-09-14 10:26:34

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when refcount is zero

On Fri, 2012-09-14 at 12:45 +0300, Andrei Emeltchenko wrote:
> Hi Maxim,
>
> On Fri, Sep 14, 2012 at 02:56:21AM +0300, Maxim Levitsky wrote:
> > On Fri, 2012-07-13 at 16:56 +0530, Jaganath wrote:
> > > Hi Andrei,
> > >
> > > --------------------------------------------------
> > > From: "Andrei Emeltchenko" <[email protected]>
> > > Sent: Friday, July 13, 2012 2:19 PM
> > > To: "Jaganath Kanakkassery" <[email protected]>
> > > Cc: <[email protected]>; <[email protected]>
> > > Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when
> > > refcount is zero
> > >
> > > > Hi Jaganath,
> > > >
> > > > On Fri, Jul 13, 2012 at 01:22:03PM +0530, Jaganath Kanakkassery wrote:
> > > >> Move the l2cap channel list chan->global_l under the refcnt
> > > >> protection and free it based on the refcnt.
> > > >
> > > > The idea is good.
> > > >
> > Note that in 3.6-rc5 which soon to be released, it trivial to trigger a
> > crash by suspending the system with a2dp headphones connected.
> > I also have seem (and I strongly suspect the same) crash in 3.5
> > This patch seems to fix this so far.
> > Could you sent it to Linus, to fix this kernel panic?
>
> AFAIK the updated patch has been applied. Concerning your crash you can
> also check my patch in the mail archive "Bluetooth: Add refcnt to l2cap_conn"
>
> Best regards
> Andrei Emeltchenko

I'll will.
Even if it is applied, its not yet in Linus's tree of today.

Thanks for your patch!

Best regards,
Maxim Levitsky

>
> >
> > Best regards,
> > Maxim Levitsky
> >
> > PS:
> > The backtrace:
> >
> >
> > fg80211: Calling CRDA to update world regulatory domain
> > cfg80211: World regulatory domain updated:
> > cfg80211: (start_freq - end_freq @ bandwidth), (max_antenna_gain,
> > max_eirp)
> > cfg80211: (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000
> > mBm)2
> > cfg80211: (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
> > cfg80211: (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000
> > mBm)T:
> > cfg80211: (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> > cfg80211: (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> > 8
> > cfg80211: World regulatory domain updated:
> > cfg80211: (start_freq - end_freq @ bandwidth), (max_antenna_gain,
> > max_eirp)
> > cfg80211: (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000
> > mBm)<
> > cfg80211: (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000
> > mBm)P@
> > cfg80211: (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
> > cfg80211: (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> > cfg80211: (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> > general protection fault: 0000 [#1] PREEMPT SMP
> > Modules linked in: hidp af_packet bnep rfcomm iwl3945
> > snd_hda_codec_realtek iwlegacy mac80211 snd_hda_intel nfsd btusb
> > snd_hda_codec uvcvideo snd_hwdep videobuf2_core snd_pcm bluetooth
> > videobuf2_vmalloc videobuf2_memops nfs_acl auth_rpcgss r592 cfg80211
> > microcode nfs psmouse memstick serio_raw snd_page_alloc ene_ir battery
> > ac lockd sunrpc rc_lirc ir_lirc_codec lirc_dev ir_rc6_decoder
> > firewire_net iTCO_wdt firewire_sbp2 nouveau ttm drm_kms_helper mxm_wmi
> > usb_storage video wmi uhci_hcd sdhci_pci firewire_ohci firewire_core
> > sdhci mmc_core atkbd ehci_hcd thermal [last unloaded: tg3]
> > CPU 0
> > Pid: 3512, comm: bluetoothd Not tainted 3.6.0-rc5+ #34 Acer Aspire
> > 5720 /Nettiling?b
> > RIP: 0010:[<ffffffffa03a1d56>] [<ffffffffa03a1d56>] l2cap_chan_destroy
> > +0x46/0xb0 [bluetooth]
> > RSP: 0018:ffff8800619dbca8 EFLAGS: 00010296
> > RAX: dead000000200200 RBX: ffff88007d3f1000 RCX: dead000000100100
> > RDX: dead000000100100 RSI: dead000000200200 RDI: ffffffffa03b7cc0
> > RBP: ffff8800619dbcb8 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0001cc0f349b43de R11: ffffffffa03a1d2d R12: ffff88007d3f1000
> > R13: ffff88007d3f1014 R14: ffff88005f922570 R15: ffff88005f9229e0
> > FS: 00007f8aa4634740(0000) GS:ffff88007fa00000(0000)
> > knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007f8aa46b7d90 CR3: 000000005f945000 CR4: 00000000000007f0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > Process bluetoothd (pid: 3512, threadinfo ffff8800619da000, task
> > ffff88005ec4bde0)
> > Stack:
> > ffff8800619dbcd8 ffff880079945800 ffff8800619dbcd8 ffffffffa03a7b3c
> > ffff88007d3f1000 ffff88005f922800 ffff8800619dbce8 ffffffffa03a7b85
> > ffff8800619dbd48 ffffffffa03a231e ffff88007d3f14a0 ffff88005f9229f0
> > Call Trace:
> > [<ffffffffa03a7b3c>] l2cap_sock_kill+0x7c/0xb0 [bluetooth]
> > [<ffffffffa03a7b85>] l2cap_sock_close_cb+0x15/0x20 [bluetooth]
> > [<ffffffffa03a231e>] l2cap_conn_del+0x11e/0x1f0 [bluetooth]
> > [<ffffffffa038301a>] ? hci_dev_do_close+0x18a/0x370 [bluetooth]
> > [<ffffffffa03a6db3>] l2cap_disconn_cfm+0x53/0x60 [bluetooth]
> > [<ffffffffa0388c55>] hci_conn_hash_flush+0x95/0x100 [bluetooth]
> > [<ffffffffa038302a>] hci_dev_do_close+0x19a/0x370 [bluetooth]
> > [<ffffffffa0384af0>] hci_dev_close+0x50/0x80 [bluetooth]
> > [<ffffffffa039af3a>] hci_sock_ioctl+0x15a/0x420 [bluetooth]
> > [<ffffffff8144c6e0>] sock_do_ioctl+0x30/0x60
> > [<ffffffff8105f3e0>] ? task_work_run+0x30/0xa0
> > [<ffffffff8144d570>] sock_ioctl+0x290/0x2b0
> > [<ffffffff81147f20>] do_vfs_ioctl+0x580/0x5e0
> > [<ffffffff8151630b>] ? _raw_spin_unlock_irq+0x3b/0x60
> > [<ffffffff81147fcf>] sys_ioctl+0x4f/0x80
> > [<ffffffff81517016>] system_call_fastpath+0x1a/0x1f
> > Code: 17 e1 48 8b 93 80 04 00 00 48 b9 00 01 10 00 00 00 ad de 48 8b 83
> > 88 04 00 00 48 c7 c7 c0 7c 3b a0 48 be 00 02 20 00 00 00 ad de <48> 89
> > 42 08 48 89 10 48 89 8b 80 04 00 00 48 89 b3 88 04 00 00
> > "RIP [<ffffffffa03a1d56>] l2cap_chan_destroy+0x46/0xb0 [bluetooth]
> > RSP <ffff8800619dbca8>
> > ---[ end trace 6e537072816e99b2 ]---+qZ
> > Kernel panic - not syncing: Fatal exception
> > fpanic occurred, switching back to text console
> > Rebooting in 10 seconds..
> > ACPI MEMORY or I/O RESET_REG.
> >

--
Best regards,
Maxim Levitsky

Visit my blog: http://maximlevitsky.wordpress.com
Warning: Above blog contains rants.


2012-09-14 09:45:00

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when refcount is zero

Hi Maxim,

On Fri, Sep 14, 2012 at 02:56:21AM +0300, Maxim Levitsky wrote:
> On Fri, 2012-07-13 at 16:56 +0530, Jaganath wrote:
> > Hi Andrei,
> >
> > --------------------------------------------------
> > From: "Andrei Emeltchenko" <[email protected]>
> > Sent: Friday, July 13, 2012 2:19 PM
> > To: "Jaganath Kanakkassery" <[email protected]>
> > Cc: <[email protected]>; <[email protected]>
> > Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when
> > refcount is zero
> >
> > > Hi Jaganath,
> > >
> > > On Fri, Jul 13, 2012 at 01:22:03PM +0530, Jaganath Kanakkassery wrote:
> > >> Move the l2cap channel list chan->global_l under the refcnt
> > >> protection and free it based on the refcnt.
> > >
> > > The idea is good.
> > >
> Note that in 3.6-rc5 which soon to be released, it trivial to trigger a
> crash by suspending the system with a2dp headphones connected.
> I also have seem (and I strongly suspect the same) crash in 3.5
> This patch seems to fix this so far.
> Could you sent it to Linus, to fix this kernel panic?

AFAIK the updated patch has been applied. Concerning your crash you can
also check my patch in the mail archive "Bluetooth: Add refcnt to l2cap_conn"

Best regards
Andrei Emeltchenko

>
> Best regards,
> Maxim Levitsky
>
> PS:
> The backtrace:
>
>
> fg80211: Calling CRDA to update world regulatory domain
> cfg80211: World regulatory domain updated:
> cfg80211: (start_freq - end_freq @ bandwidth), (max_antenna_gain,
> max_eirp)
> cfg80211: (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000
> mBm)2
> cfg80211: (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
> cfg80211: (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000
> mBm)T:
> cfg80211: (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> cfg80211: (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> 8
> cfg80211: World regulatory domain updated:
> cfg80211: (start_freq - end_freq @ bandwidth), (max_antenna_gain,
> max_eirp)
> cfg80211: (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000
> mBm)<
> cfg80211: (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000
> mBm)P@
> cfg80211: (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
> cfg80211: (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> cfg80211: (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> general protection fault: 0000 [#1] PREEMPT SMP
> Modules linked in: hidp af_packet bnep rfcomm iwl3945
> snd_hda_codec_realtek iwlegacy mac80211 snd_hda_intel nfsd btusb
> snd_hda_codec uvcvideo snd_hwdep videobuf2_core snd_pcm bluetooth
> videobuf2_vmalloc videobuf2_memops nfs_acl auth_rpcgss r592 cfg80211
> microcode nfs psmouse memstick serio_raw snd_page_alloc ene_ir battery
> ac lockd sunrpc rc_lirc ir_lirc_codec lirc_dev ir_rc6_decoder
> firewire_net iTCO_wdt firewire_sbp2 nouveau ttm drm_kms_helper mxm_wmi
> usb_storage video wmi uhci_hcd sdhci_pci firewire_ohci firewire_core
> sdhci mmc_core atkbd ehci_hcd thermal [last unloaded: tg3]
> CPU 0
> Pid: 3512, comm: bluetoothd Not tainted 3.6.0-rc5+ #34 Acer Aspire
> 5720 /Nettiling?b
> RIP: 0010:[<ffffffffa03a1d56>] [<ffffffffa03a1d56>] l2cap_chan_destroy
> +0x46/0xb0 [bluetooth]
> RSP: 0018:ffff8800619dbca8 EFLAGS: 00010296
> RAX: dead000000200200 RBX: ffff88007d3f1000 RCX: dead000000100100
> RDX: dead000000100100 RSI: dead000000200200 RDI: ffffffffa03b7cc0
> RBP: ffff8800619dbcb8 R08: 0000000000000000 R09: 0000000000000000
> R10: 0001cc0f349b43de R11: ffffffffa03a1d2d R12: ffff88007d3f1000
> R13: ffff88007d3f1014 R14: ffff88005f922570 R15: ffff88005f9229e0
> FS: 00007f8aa4634740(0000) GS:ffff88007fa00000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f8aa46b7d90 CR3: 000000005f945000 CR4: 00000000000007f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process bluetoothd (pid: 3512, threadinfo ffff8800619da000, task
> ffff88005ec4bde0)
> Stack:
> ffff8800619dbcd8 ffff880079945800 ffff8800619dbcd8 ffffffffa03a7b3c
> ffff88007d3f1000 ffff88005f922800 ffff8800619dbce8 ffffffffa03a7b85
> ffff8800619dbd48 ffffffffa03a231e ffff88007d3f14a0 ffff88005f9229f0
> Call Trace:
> [<ffffffffa03a7b3c>] l2cap_sock_kill+0x7c/0xb0 [bluetooth]
> [<ffffffffa03a7b85>] l2cap_sock_close_cb+0x15/0x20 [bluetooth]
> [<ffffffffa03a231e>] l2cap_conn_del+0x11e/0x1f0 [bluetooth]
> [<ffffffffa038301a>] ? hci_dev_do_close+0x18a/0x370 [bluetooth]
> [<ffffffffa03a6db3>] l2cap_disconn_cfm+0x53/0x60 [bluetooth]
> [<ffffffffa0388c55>] hci_conn_hash_flush+0x95/0x100 [bluetooth]
> [<ffffffffa038302a>] hci_dev_do_close+0x19a/0x370 [bluetooth]
> [<ffffffffa0384af0>] hci_dev_close+0x50/0x80 [bluetooth]
> [<ffffffffa039af3a>] hci_sock_ioctl+0x15a/0x420 [bluetooth]
> [<ffffffff8144c6e0>] sock_do_ioctl+0x30/0x60
> [<ffffffff8105f3e0>] ? task_work_run+0x30/0xa0
> [<ffffffff8144d570>] sock_ioctl+0x290/0x2b0
> [<ffffffff81147f20>] do_vfs_ioctl+0x580/0x5e0
> [<ffffffff8151630b>] ? _raw_spin_unlock_irq+0x3b/0x60
> [<ffffffff81147fcf>] sys_ioctl+0x4f/0x80
> [<ffffffff81517016>] system_call_fastpath+0x1a/0x1f
> Code: 17 e1 48 8b 93 80 04 00 00 48 b9 00 01 10 00 00 00 ad de 48 8b 83
> 88 04 00 00 48 c7 c7 c0 7c 3b a0 48 be 00 02 20 00 00 00 ad de <48> 89
> 42 08 48 89 10 48 89 8b 80 04 00 00 48 89 b3 88 04 00 00
> "RIP [<ffffffffa03a1d56>] l2cap_chan_destroy+0x46/0xb0 [bluetooth]
> RSP <ffff8800619dbca8>
> ---[ end trace 6e537072816e99b2 ]---+qZ
> Kernel panic - not syncing: Fatal exception
> fpanic occurred, switching back to text console
> Rebooting in 10 seconds..
> ACPI MEMORY or I/O RESET_REG.
>

2012-09-13 23:56:21

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when refcount is zero

On Fri, 2012-07-13 at 16:56 +0530, Jaganath wrote:
> Hi Andrei,
>
> --------------------------------------------------
> From: "Andrei Emeltchenko" <[email protected]>
> Sent: Friday, July 13, 2012 2:19 PM
> To: "Jaganath Kanakkassery" <[email protected]>
> Cc: <[email protected]>; <[email protected]>
> Subject: Re: [PATCH] Bluetooth: Free the l2cap channel list only when
> refcount is zero
>
> > Hi Jaganath,
> >
> > On Fri, Jul 13, 2012 at 01:22:03PM +0530, Jaganath Kanakkassery wrote:
> >> Move the l2cap channel list chan->global_l under the refcnt
> >> protection and free it based on the refcnt.
> >
> > The idea is good.
> >
Note that in 3.6-rc5 which soon to be released, it trivial to trigger a
crash by suspending the system with a2dp headphones connected.
I also have seem (and I strongly suspect the same) crash in 3.5
This patch seems to fix this so far.
Could you sent it to Linus, to fix this kernel panic?

Best regards,
Maxim Levitsky

PS:
The backtrace:


fg80211: Calling CRDA to update world regulatory domain
cfg80211: World regulatory domain updated:
cfg80211: (start_freq - end_freq @ bandwidth), (max_antenna_gain,
max_eirp)
cfg80211: (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000
mBm)2
cfg80211: (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
cfg80211: (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000
mBm)T:
cfg80211: (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
cfg80211: (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
8
cfg80211: World regulatory domain updated:
cfg80211: (start_freq - end_freq @ bandwidth), (max_antenna_gain,
max_eirp)
cfg80211: (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000
mBm)<
cfg80211: (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000
mBm)P@
cfg80211: (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
cfg80211: (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
cfg80211: (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
general protection fault: 0000 [#1] PREEMPT SMP
Modules linked in: hidp af_packet bnep rfcomm iwl3945
snd_hda_codec_realtek iwlegacy mac80211 snd_hda_intel nfsd btusb
snd_hda_codec uvcvideo snd_hwdep videobuf2_core snd_pcm bluetooth
videobuf2_vmalloc videobuf2_memops nfs_acl auth_rpcgss r592 cfg80211
microcode nfs psmouse memstick serio_raw snd_page_alloc ene_ir battery
ac lockd sunrpc rc_lirc ir_lirc_codec lirc_dev ir_rc6_decoder
firewire_net iTCO_wdt firewire_sbp2 nouveau ttm drm_kms_helper mxm_wmi
usb_storage video wmi uhci_hcd sdhci_pci firewire_ohci firewire_core
sdhci mmc_core atkbd ehci_hcd thermal [last unloaded: tg3]
CPU 0
Pid: 3512, comm: bluetoothd Not tainted 3.6.0-rc5+ #34 Acer Aspire
5720 /Nettiling?b
RIP: 0010:[<ffffffffa03a1d56>] [<ffffffffa03a1d56>] l2cap_chan_destroy
+0x46/0xb0 [bluetooth]
RSP: 0018:ffff8800619dbca8 EFLAGS: 00010296
RAX: dead000000200200 RBX: ffff88007d3f1000 RCX: dead000000100100
RDX: dead000000100100 RSI: dead000000200200 RDI: ffffffffa03b7cc0
RBP: ffff8800619dbcb8 R08: 0000000000000000 R09: 0000000000000000
R10: 0001cc0f349b43de R11: ffffffffa03a1d2d R12: ffff88007d3f1000
R13: ffff88007d3f1014 R14: ffff88005f922570 R15: ffff88005f9229e0
FS: 00007f8aa4634740(0000) GS:ffff88007fa00000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f8aa46b7d90 CR3: 000000005f945000 CR4: 00000000000007f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process bluetoothd (pid: 3512, threadinfo ffff8800619da000, task
ffff88005ec4bde0)
Stack:
ffff8800619dbcd8 ffff880079945800 ffff8800619dbcd8 ffffffffa03a7b3c
ffff88007d3f1000 ffff88005f922800 ffff8800619dbce8 ffffffffa03a7b85
ffff8800619dbd48 ffffffffa03a231e ffff88007d3f14a0 ffff88005f9229f0
Call Trace:
[<ffffffffa03a7b3c>] l2cap_sock_kill+0x7c/0xb0 [bluetooth]
[<ffffffffa03a7b85>] l2cap_sock_close_cb+0x15/0x20 [bluetooth]
[<ffffffffa03a231e>] l2cap_conn_del+0x11e/0x1f0 [bluetooth]
[<ffffffffa038301a>] ? hci_dev_do_close+0x18a/0x370 [bluetooth]
[<ffffffffa03a6db3>] l2cap_disconn_cfm+0x53/0x60 [bluetooth]
[<ffffffffa0388c55>] hci_conn_hash_flush+0x95/0x100 [bluetooth]
[<ffffffffa038302a>] hci_dev_do_close+0x19a/0x370 [bluetooth]
[<ffffffffa0384af0>] hci_dev_close+0x50/0x80 [bluetooth]
[<ffffffffa039af3a>] hci_sock_ioctl+0x15a/0x420 [bluetooth]
[<ffffffff8144c6e0>] sock_do_ioctl+0x30/0x60
[<ffffffff8105f3e0>] ? task_work_run+0x30/0xa0
[<ffffffff8144d570>] sock_ioctl+0x290/0x2b0
[<ffffffff81147f20>] do_vfs_ioctl+0x580/0x5e0
[<ffffffff8151630b>] ? _raw_spin_unlock_irq+0x3b/0x60
[<ffffffff81147fcf>] sys_ioctl+0x4f/0x80
[<ffffffff81517016>] system_call_fastpath+0x1a/0x1f
Code: 17 e1 48 8b 93 80 04 00 00 48 b9 00 01 10 00 00 00 ad de 48 8b 83
88 04 00 00 48 c7 c7 c0 7c 3b a0 48 be 00 02 20 00 00 00 ad de <48> 89
42 08 48 89 10 48 89 8b 80 04 00 00 48 89 b3 88 04 00 00
"RIP [<ffffffffa03a1d56>] l2cap_chan_destroy+0x46/0xb0 [bluetooth]
RSP <ffff8800619dbca8>
---[ end trace 6e537072816e99b2 ]---+qZ
Kernel panic - not syncing: Fatal exception
fpanic occurred, switching back to text console
Rebooting in 10 seconds..
ACPI MEMORY or I/O RESET_REG.