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

[JÃrg Otte]

2015-06-29 23:13 GMT+02:00 Tedd Ho-Jeong An <tedd.an@xxxxxxxxx>:
> 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

I now have both patche applied and no revert. Looks good so far.

Thanks, JÃrg
--
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/