From: Jeffy Chen <jeffy.chen@rock-chips.com>
To: linux-bluetooth@vger.kernel.org
Cc: Brian Norris <briannorris@chromium.org>,
Douglas Anderson <dianders@chromium.org>,
Johan Hedberg <johan.hedberg@intel.com>,
Peter Hurley <peter@hurleysoftware.com>,
Jeffy Chen <jeffy.chen@rock-chips.com>,
Johan Hedberg <johan.hedberg@gmail.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
"David S. Miller" <davem@davemloft.net>,
Marcel Holtmann <marcel@holtmann.org>,
Gustavo Padovan <gustavo@padovan.org>
Subject: [PATCH v3 3/3] Bluetooth: hidp: fix possible might sleep error in hidp_session_thread
Date: Mon, 13 Feb 2017 12:12:34 +0800 [thread overview]
Message-ID: <1486959154-25810-3-git-send-email-jeffy.chen@rock-chips.com> (raw)
In-Reply-To: <1486959154-25810-1-git-send-email-jeffy.chen@rock-chips.com>
It looks like hidp_session_thread 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>
1/ Fix could not wake up by wake attempts on original wait queues.
2/ Remove unnecessary memory barrier before wake_up_* functions.
---
Changes in v3: None
Changes in v2: None
net/bluetooth/hidp/core.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 0bec458..076bc50 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -36,6 +36,7 @@
#define VERSION "1.2"
static DECLARE_RWSEM(hidp_session_sem);
+static DECLARE_WAIT_QUEUE_HEAD(hidp_session_wq);
static LIST_HEAD(hidp_session_list);
static unsigned char hidp_keycode[256] = {
@@ -1068,12 +1069,12 @@ static int hidp_session_start_sync(struct hidp_session *session)
* Wake up session thread and notify it to stop. This is asynchronous and
* returns immediately. Call this whenever a runtime error occurs and you want
* the session to stop.
- * Note: wake_up_process() performs any necessary memory-barriers for us.
+ * Note: wake_up_interruptible() performs any necessary memory-barriers for us.
*/
static void hidp_session_terminate(struct hidp_session *session)
{
atomic_inc(&session->terminate);
- wake_up_process(session->task);
+ wake_up_interruptible(&hidp_session_wq);
}
/*
@@ -1180,7 +1181,9 @@ static void hidp_session_run(struct hidp_session *session)
struct sock *ctrl_sk = session->ctrl_sock->sk;
struct sock *intr_sk = session->intr_sock->sk;
struct sk_buff *skb;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ add_wait_queue(&hidp_session_wq, &wait);
for (;;) {
/*
* This thread can be woken up two ways:
@@ -1188,12 +1191,10 @@ static void hidp_session_run(struct hidp_session *session)
* session->terminate flag and wakes this thread up.
* - Via modifying the socket state of ctrl/intr_sock. This
* thread is woken up by ->sk_state_changed().
- *
- * Note: set_current_state() performs any necessary
- * memory-barriers for us.
*/
- set_current_state(TASK_INTERRUPTIBLE);
+ /* Ensure session->terminate is updated */
+ smp_mb__before_atomic();
if (atomic_read(&session->terminate))
break;
@@ -1227,11 +1228,22 @@ static void hidp_session_run(struct hidp_session *session)
hidp_process_transmit(session, &session->ctrl_transmit,
session->ctrl_sock);
- schedule();
+ wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
}
+ remove_wait_queue(&hidp_session_wq, &wait);
atomic_inc(&session->terminate);
- set_current_state(TASK_RUNNING);
+
+ /* Ensure session->terminate is updated */
+ smp_mb__after_atomic();
+}
+
+int hidp_session_wake_function(wait_queue_t *wait, unsigned int mode,
+ int sync, void *key)
+{
+ wake_up_interruptible(&hidp_session_wq);
+
+ return default_wake_function(wait, mode, sync, key);
}
/*
@@ -1244,7 +1256,8 @@ static void hidp_session_run(struct hidp_session *session)
static int hidp_session_thread(void *arg)
{
struct hidp_session *session = arg;
- wait_queue_t ctrl_wait, intr_wait;
+ DEFINE_WAIT_FUNC(ctrl_wait, hidp_session_wake_function);
+ DEFINE_WAIT_FUNC(intr_wait, hidp_session_wake_function);
BT_DBG("session %p", session);
@@ -1254,8 +1267,6 @@ static int hidp_session_thread(void *arg)
set_user_nice(current, -15);
hidp_set_timer(session);
- init_waitqueue_entry(&ctrl_wait, current);
- init_waitqueue_entry(&intr_wait, current);
add_wait_queue(sk_sleep(session->ctrl_sock->sk), &ctrl_wait);
add_wait_queue(sk_sleep(session->intr_sock->sk), &intr_wait);
/* This memory barrier is paired with wq_has_sleeper(). See
--
2.1.4
next prev parent reply other threads:[~2017-02-13 4:12 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-13 4:12 [PATCH v3 1/3] Bluetooth: bnep: fix possible might sleep error in bnep_session Jeffy Chen
2017-02-13 4:12 ` [PATCH v3 2/3] Bluetooth: cmtp: fix possible might sleep error in cmtp_session Jeffy Chen
2017-02-13 4:12 ` Jeffy Chen [this message]
2017-02-13 21:27 ` [PATCH v3 3/3] Bluetooth: hidp: fix possible might sleep error in hidp_session_thread Dmitry Torokhov
2017-02-14 0:57 ` jeffy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1486959154-25810-3-git-send-email-jeffy.chen@rock-chips.com \
--to=jeffy.chen@rock-chips.com \
--cc=briannorris@chromium.org \
--cc=davem@davemloft.net \
--cc=dianders@chromium.org \
--cc=gustavo@padovan.org \
--cc=johan.hedberg@gmail.com \
--cc=johan.hedberg@intel.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcel@holtmann.org \
--cc=netdev@vger.kernel.org \
--cc=peter@hurleysoftware.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).