Hi Jorg
On Mon, 29 Jun 2015 16:37:32 +0200
Jörg Otte <jrg.otte@xxxxxxxxx> wrote:
> 2015-06-29 12:30 GMT+02:00 Alexey Dobriyan <adobriyan@xxxxxxxxx>:
> > On Mon, Jun 29, 2015 at 12:00 PM, Jörg Otte <jrg.otte@xxxxxxxxx> wrote:
> >> 2015-06-28 18:09 GMT+02:00 Alexey Dobriyan <adobriyan@xxxxxxxxx>:
> >>> On Sun, Jun 28, 2015 at 05:36:04PM +0200, Jörg Otte wrote:
> >>>> 2015-06-26 16:28 GMT+02:00 Jörg Otte <jrg.otte@xxxxxxxxx>:
> >>>> > 2015-06-26 12:03 GMT+02:00 Jörg Otte <jrg.otte@xxxxxxxxx>:
> >>>> >> 2015-06-26 11:37 GMT+02:00 Marcel Holtmann <marcel@xxxxxxxxxxxx>:
> >>>> >>> Hi Joerg,
> >>>> >>>
> >>>> >>>> Bluetooth is inoperable in current Linus tree and the
> >>>> >>>> first bad commit is:
> >>>> >>>>
> >>>> >>>> 835a6a2f8603237a3e6cded5a6765090ecb06ea5 is the first bad commit
> >>>> >>>> commit 835a6a2f8603237a3e6cded5a6765090ecb06ea5
> >>>> >>>> Author: Alexey Dobriyan <adobriyan@xxxxxxxxx>
> >>>> >>>> Date: Wed Jun 10 20:28:33 2015 +0300
> >>>> >>>>
> >>>> >>>> Bluetooth: Stop sabotaging list poisoning
> >>>> >>>>
> >>>> >>>> list_del() poisons pointers with special values, no need to overwrite them.
> >>>> >>>>
> >>>> >>>> Signed-off-by: Alexey Dobriyan <adobriyan@xxxxxxxxx>
> >>>> >>>> Signed-off-by: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> >>>> >>>>
> >>>> >>>> My BT adapter is an intel 8087:07da
> >>>> >>>> I reverted that commit and this fixed the problem for me.
> >>>> >>>
> >>>> >>> today we had a patch from Tedd fixing the list initialization in the HIDP code.
> >>>> >>>
> >>>> >>> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> >>>> >>> index 9070dfd6b4ad..f1a117f8cad2 100644
> >>>> >>> --- a/net/bluetooth/hidp/core.c
> >>>> >>> +++ b/net/bluetooth/hidp/core.c
> >>>> >>> @@ -915,6 +915,7 @@ static int hidp_session_new(struct hidp_session **out, const bdaddr_t *bdaddr,
> >>>> >>> session->conn = l2cap_conn_get(conn);
> >>>> >>> session->user.probe = hidp_session_probe;
> >>>> >>> session->user.remove = hidp_session_remove;
> >>>> >>> + INIT_LIST_HEAD(&session->user.list);
> >>>> >>> session->ctrl_sock = ctrl_sock;
> >>>> >>> session->intr_sock = intr_sock;
> >>>> >>> skb_queue_head_init(&session->ctrl_transmit);
> >>>> >>>
> >>>> >>> Could this be fixing it for you as well?
> >>>> >>>
> >>>> >> I will check this when I am at home in the
> >>>> >> afternoon.
> >>>> >>
> >>>> >
> >>>> > The patch works for me too.
> >>>> >
> >>>> Ok, this was a little bit hasty!
> >>>> I now see the following additional problems:
> >>>>
> >>>> - System freeze on resume (occures always).
> >>>> - System freeze on shutdown (occures sometimes)
> >>>> - System freeze when BT-mouse is connecting (occures sometimes).
> >>>>
> >>>> Then I can't do anything except power off.
> >>>>
> >>>> This happens only if Bluetooth AND BT-mouse is activated.
> >>>
> >>> OK, what happens if you just revert only list_del patch?
> >>
> >> I have applied this patch:
> >>
> >> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> >> index 9070dfd6b4ad..f1a117f8cad2 100644
> >> --- a/net/bluetooth/hidp/core.c
> >> +++ b/net/bluetooth/hidp/core.c
> >> @@ -915,6 +915,7 @@ static int hidp_session_new(struct hidp_session
> >> **out, const bdaddr_t *bdaddr,
> >> session->conn = l2cap_conn_get(conn);
> >> session->user.probe = hidp_session_probe;
> >> session->user.remove = hidp_session_remove;
> >> + INIT_LIST_HEAD(&session->user.list);
> >> session->ctrl_sock = ctrl_sock;
> >> session->intr_sock = intr_sock;
> >> skb_queue_head_init(&session->ctrl_transmit);
> >>
> >> without this patch bluetooth doesn't work at all for me.
> >
> > Sure.
> >
> > Please drop this patch, and do
> >
> > git-revert 835a6a2f8603237a3e6cded5a6765090ecb06ea5
> >
> > Maybe it's some other changes causing hangs.
>
> Looks good so far. The system freeze on resume is gone.
>
> Thanks, Jörg
Regarding the system hang issue, it looks like the problem is caused by the list_del().
According to the list.h, this macro puts the entry into invalid state and it causes the device hang in the l2cap_core.c
/**
* list_del - deletes entry from list.
* @entry: the element to delete from the list.
* Note: list_empty() on entry does not return true after this, the entry is
* in an undefined state.
*/
So, one way to fix this issue is using the list_del_init() instead.
Can you try this patch to see if it resolve the issue? No need to revert any patch.
I ran a quick test with a different scenarios and it looks good to me so far.
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 51594fb..45fffa4 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1634,7 +1634,7 @@ void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user *user)
if (list_empty(&user->list))
goto out_unlock;
- list_del(&user->list);
+ list_del_init(&user->list);
user->remove(conn, user);
out_unlock:
@@ -1648,7 +1648,7 @@ static void l2cap_unregister_all_users(struct l2cap_conn *conn)
while (!list_empty(&conn->users)) {
user = list_first_entry(&conn->users, struct l2cap_user, list);
- list_del(&user->list);
+ list_del_init(&user->list);
user->remove(conn, user);
}
}
Regards,
Tedd Ho-Jeong An
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
