| Message ID | 1498556084-25346-1-git-send-email-jeffy.chen@rock-chips.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi Jeffy, > It looks like bnep_session has same pattern as the issue reported in > old rfcomm: > > while (1) { > set_current_state(TASK_INTERRUPTIBLE); > if (condition) > break; > // may call might_sleep here > schedule(); > } > __set_current_state(TASK_RUNNING); > > Which fixed at: > dfb2fae Bluetooth: Fix nested sleeps > > So let's fix it at the same way, also follow the suggestion of: > https://lwn.net/Articles/628628/ > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Reviewed-by: Brian Norris <briannorris@chromium.org> > Reviewed-by: AL Yu-Chen Cho <acho@suse.com> > --- > > Changes in v4: None > Changes in v2: None > > net/bluetooth/bnep/core.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) all 3 patches have been applied to bluetooth-next tree. Regards Marcel
On 06/27/2017, 07:32 PM, Marcel Holtmann wrote: >> It looks like bnep_session has same pattern as the issue reported in >> old rfcomm: >> >> while (1) { >> set_current_state(TASK_INTERRUPTIBLE); >> if (condition) >> break; >> // may call might_sleep here >> schedule(); >> } >> __set_current_state(TASK_RUNNING); >> >> Which fixed at: >> dfb2fae Bluetooth: Fix nested sleeps >> >> So let's fix it at the same way, also follow the suggestion of: >> https://lwn.net/Articles/628628/ ... > all 3 patches have been applied to bluetooth-next tree. Hi, given users are hitting it in at least 4.4 and 4.12, can we have all three in all stables where this applies? 5da8e47d849d Bluetooth: hidp: fix possible might sleep error in hidp_session_thread f06d977309d0 Bluetooth: cmtp: fix possible might sleep error in cmtp_session 25717382c1dd Bluetooth: bnep: fix possible might sleep error in bnep_session I am not sure: to stable directly or via net stable? thanks,
From: Jiri Slaby <jslaby@suse.cz> Date: Wed, 23 Aug 2017 09:29:44 +0200 > On 06/27/2017, 07:32 PM, Marcel Holtmann wrote: >>> It looks like bnep_session has same pattern as the issue reported in >>> old rfcomm: >>> >>> while (1) { >>> set_current_state(TASK_INTERRUPTIBLE); >>> if (condition) >>> break; >>> // may call might_sleep here >>> schedule(); >>> } >>> __set_current_state(TASK_RUNNING); >>> >>> Which fixed at: >>> dfb2fae Bluetooth: Fix nested sleeps >>> >>> So let's fix it at the same way, also follow the suggestion of: >>> https://lwn.net/Articles/628628/ > > ... > >> all 3 patches have been applied to bluetooth-next tree. > > Hi, > > given users are hitting it in at least 4.4 and 4.12, can we have all > three in all stables where this applies? > > 5da8e47d849d Bluetooth: hidp: fix possible might sleep error in > hidp_session_thread > f06d977309d0 Bluetooth: cmtp: fix possible might sleep error in cmtp_session > 25717382c1dd Bluetooth: bnep: fix possible might sleep error in bnep_session > > I am not sure: to stable directly or via net stable? I generally let the wireless family handle their own -stable submissions and that includes bluetooth.
Hi Jiri, >>> It looks like bnep_session has same pattern as the issue reported in >>> old rfcomm: >>> >>> while (1) { >>> set_current_state(TASK_INTERRUPTIBLE); >>> if (condition) >>> break; >>> // may call might_sleep here >>> schedule(); >>> } >>> __set_current_state(TASK_RUNNING); >>> >>> Which fixed at: >>> dfb2fae Bluetooth: Fix nested sleeps >>> >>> So let's fix it at the same way, also follow the suggestion of: >>> https://lwn.net/Articles/628628/ > > ... > >> all 3 patches have been applied to bluetooth-next tree. > > Hi, > > given users are hitting it in at least 4.4 and 4.12, can we have all > three in all stables where this applies? > > 5da8e47d849d Bluetooth: hidp: fix possible might sleep error in > hidp_session_thread > f06d977309d0 Bluetooth: cmtp: fix possible might sleep error in cmtp_session > 25717382c1dd Bluetooth: bnep: fix possible might sleep error in bnep_session > > I am not sure: to stable directly or via net stable? as Dave said, just email -stable directly and have Greg pick them up. Regards Marcel
On Wed, Aug 23, 2017 at 08:14:15PM +0200, Marcel Holtmann wrote: > Hi Jiri, > > >>> It looks like bnep_session has same pattern as the issue reported in > >>> old rfcomm: > >>> > >>> while (1) { > >>> set_current_state(TASK_INTERRUPTIBLE); > >>> if (condition) > >>> break; > >>> // may call might_sleep here > >>> schedule(); > >>> } > >>> __set_current_state(TASK_RUNNING); > >>> > >>> Which fixed at: > >>> dfb2fae Bluetooth: Fix nested sleeps > >>> > >>> So let's fix it at the same way, also follow the suggestion of: > >>> https://lwn.net/Articles/628628/ > > > > ... > > > >> all 3 patches have been applied to bluetooth-next tree. > > > > Hi, > > > > given users are hitting it in at least 4.4 and 4.12, can we have all > > three in all stables where this applies? > > > > 5da8e47d849d Bluetooth: hidp: fix possible might sleep error in > > hidp_session_thread > > f06d977309d0 Bluetooth: cmtp: fix possible might sleep error in cmtp_session > > 25717382c1dd Bluetooth: bnep: fix possible might sleep error in bnep_session > > > > I am not sure: to stable directly or via net stable? > > as Dave said, just email -stable directly and have Greg pick them up. All now picked up :)
diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c index fbf251f..4d6b94d 100644 --- a/net/bluetooth/bnep/core.c +++ b/net/bluetooth/bnep/core.c @@ -484,16 +484,16 @@ static int bnep_session(void *arg) struct net_device *dev = s->dev; struct sock *sk = s->sock->sk; struct sk_buff *skb; - wait_queue_t wait; + DEFINE_WAIT_FUNC(wait, woken_wake_function); BT_DBG(""); set_user_nice(current, -15); - init_waitqueue_entry(&wait, current); add_wait_queue(sk_sleep(sk), &wait); while (1) { - set_current_state(TASK_INTERRUPTIBLE); + /* Ensure session->terminate is updated */ + smp_mb__before_atomic(); if (atomic_read(&s->terminate)) break; @@ -515,9 +515,8 @@ static int bnep_session(void *arg) break; netif_wake_queue(dev); - schedule(); + wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT); } - __set_current_state(TASK_RUNNING); remove_wait_queue(sk_sleep(sk), &wait); /* Cleanup session */ @@ -666,7 +665,7 @@ int bnep_del_connection(struct bnep_conndel_req *req) s = __bnep_get_session(req->dst); if (s) { atomic_inc(&s->terminate); - wake_up_process(s->task); + wake_up_interruptible(sk_sleep(s->sock->sk)); } else err = -ENOENT;
It looks like bnep_session has same pattern as the issue reported in old rfcomm: while (1) { set_current_state(TASK_INTERRUPTIBLE); if (condition) break; // may call might_sleep here schedule(); } __set_current_state(TASK_RUNNING); Which fixed at: dfb2fae Bluetooth: Fix nested sleeps So let's fix it at the same way, also follow the suggestion of: https://lwn.net/Articles/628628/ Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> Reviewed-by: Brian Norris <briannorris@chromium.org> Reviewed-by: AL Yu-Chen Cho <acho@suse.com> --- Changes in v4: None Changes in v2: None net/bluetooth/bnep/core.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)