Re: [4.1.0-07254-gc13c810] Regression: Bluetooth not working.

[Tedd Ho-Jeong An]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/