Skip to content

Commit

Permalink
Bluetooth: Fix hidp disconnect deadlocks and lost wakeup
Browse files Browse the repository at this point in the history
Partial revert of commit aabf6f8. When the hidp session thread
was converted from kernel_thread to kthread, the atomic/wakeups
were replaced with kthread_stop. kthread_stop has blocking semantics
which are inappropriate for the hidp session kthread. In addition,
the kthread signals itself to terminate in hidp_process_hid_control()
- it cannot do this with kthread_stop().

Lastly, a wakeup can be lost if the wakeup happens between checking
for the loop exit condition and setting the current state to
TASK_INTERRUPTIBLE. (Without appropriate synchronization mechanisms,
the task state should not be changed between the condition test and
the yield - via schedule() - as this creates a race between the
wakeup and resetting the state back to interruptible.)

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
  • Loading branch information
Peter Hurley authored and Gustavo F. Padovan committed Jun 30, 2011
1 parent 7ac2881 commit 7bb59df
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
18 changes: 11 additions & 7 deletions net/bluetooth/hidp/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,8 @@ static void hidp_idle_timeout(unsigned long arg)
{
struct hidp_session *session = (struct hidp_session *) arg;

kthread_stop(session->task);
atomic_inc(&session->terminate);
wake_up_process(session->task);
}

static void hidp_set_timer(struct hidp_session *session)
Expand Down Expand Up @@ -535,7 +536,8 @@ static void hidp_process_hid_control(struct hidp_session *session,
skb_queue_purge(&session->ctrl_transmit);
skb_queue_purge(&session->intr_transmit);

kthread_stop(session->task);
atomic_inc(&session->terminate);
wake_up_process(current);
}
}

Expand Down Expand Up @@ -706,9 +708,8 @@ static int hidp_session(void *arg)
add_wait_queue(sk_sleep(intr_sk), &intr_wait);
session->waiting_for_startup = 0;
wake_up_interruptible(&session->startup_queue);
while (!kthread_should_stop()) {
set_current_state(TASK_INTERRUPTIBLE);

set_current_state(TASK_INTERRUPTIBLE);
while (!atomic_read(&session->terminate)) {
if (ctrl_sk->sk_state != BT_CONNECTED ||
intr_sk->sk_state != BT_CONNECTED)
break;
Expand All @@ -726,6 +727,7 @@ static int hidp_session(void *arg)
hidp_process_transmit(session);

schedule();
set_current_state(TASK_INTERRUPTIBLE);
}
set_current_state(TASK_RUNNING);
remove_wait_queue(sk_sleep(intr_sk), &intr_wait);
Expand Down Expand Up @@ -1060,7 +1062,8 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
err_add_device:
hid_destroy_device(session->hid);
session->hid = NULL;
kthread_stop(session->task);
atomic_inc(&session->terminate);
wake_up_process(session->task);

unlink:
hidp_del_timer(session);
Expand Down Expand Up @@ -1111,7 +1114,8 @@ int hidp_del_connection(struct hidp_conndel_req *req)
skb_queue_purge(&session->ctrl_transmit);
skb_queue_purge(&session->intr_transmit);

kthread_stop(session->task);
atomic_inc(&session->terminate);
wake_up_process(session->task);
}
} else
err = -ENOENT;
Expand Down
1 change: 1 addition & 0 deletions net/bluetooth/hidp/hidp.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ struct hidp_session {
uint ctrl_mtu;
uint intr_mtu;

atomic_t terminate;
struct task_struct *task;

unsigned char keys[8];
Expand Down

0 comments on commit 7bb59df

Please sign in to comment.