netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 3/3] Bluetooth: hidp: fix possible might sleep error in hidp_session_thread
Date: Tue, 24 Jan 2017 12:07:51 +0800	[thread overview]
Message-ID: <1485230871-22828-3-git-send-email-jeffy.chen@rock-chips.com> (raw)
In-Reply-To: <1485230871-22828-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>
---

 net/bluetooth/hidp/core.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 0bec458..43d6e6a 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,15 @@ 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.
  */
 static void hidp_session_terminate(struct hidp_session *session)
 {
 	atomic_inc(&session->terminate);
-	wake_up_process(session->task);
+
+	/* Ensure session->terminate is updated */
+	smp_mb__after_atomic();
+
+	wake_up_interruptible(&hidp_session_wq);
 }
 
 /*
@@ -1180,7 +1184,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 +1194,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 +1231,14 @@ 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();
 }
 
 /*
-- 
2.1.4

  parent reply	other threads:[~2017-01-24  4:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-24  4:07 [PATCH 1/3] Bluetooth: bnep: fix possible might sleep error in bnep_session Jeffy Chen
2017-01-24  4:07 ` [PATCH 2/3] Bluetooth: cmtp: fix possible might sleep error in cmtp_session Jeffy Chen
2017-02-11  1:43   ` Brian Norris
2017-02-13  4:14     ` jeffy
2017-01-24  4:07 ` Jeffy Chen [this message]
     [not found]   ` <1485230871-22828-3-git-send-email-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2017-02-11  1:26     ` [PATCH 3/3] Bluetooth: hidp: fix possible might sleep error in hidp_session_thread Brian Norris
2017-02-13  4:16       ` jeffy
2017-02-11  1:40 ` [PATCH 1/3] Bluetooth: bnep: fix possible might sleep error in bnep_session Brian Norris
2017-02-13  4:13   ` 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=1485230871-22828-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).