On Thu, Oct 02, 2014 at 01:09:27PM +0200, Peter Zijlstra wrote:
> On Tue, Sep 30, 2014 at 04:02:28PM +0800, Fengguang Wu wrote:
> > Hi Peter,
> >
> > We possibly find a rfcomm bug (maintainers CCed) exposed by your debug patch
> >
> > [ 1.861895] NET: Registered protocol family 5
> > [ 1.862978] Bluetooth: RFCOMM TTY layer initialized
> > [ 1.863099] ------------[ cut here ]------------
> > [ 1.863105] WARNING: CPU: 1 PID: 79 at kernel/sched/core.c:7156
> > __might_sleep+0x17d/0x1a1()
> > [ 1.863112] do not call blocking ops when !TASK_RUNNING; state=1 set at
> > [<c14dc381>] rfcomm_run+0xdf/0x130e
> > [ 1.863591] [<c1058b73>] ? kthread_stop+0x53/0x53
> > [ 1.864906] [<c155a411>] dump_stack+0x48/0x60
> > [ 1.866298] [<c14dc381>] ? rfcomm_run+0xdf/0x130e
>
> Ha yes, rfcomm_run is a complete buggy mess indeed. Lemme go see what I
> can make of it.
---
Subject: rfcomm: Fix broken wait construct
rfcomm_run() is a tad broken in that is has a nested wait loop. One
cannot rely on p->state for the outer wait because the inner wait will
overwrite it.
While at it, rename rfcomm_schedule to rfcomm_wake, since that is what
it actually does.
Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
---
net/bluetooth/rfcomm/core.c | 46 +++++++++++++++++++++-----------------------
1 file changed, 22 insertions(+), 24 deletions(-)
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -101,11 +101,11 @@ static struct rfcomm_session *rfcomm_ses
#define __get_rpn_stop_bits(line) (((line) >> 2) & 0x1)
#define __get_rpn_parity(line) (((line) >> 3) & 0x7)
-static void rfcomm_schedule(void)
+static DECLARE_WAIT_QUEUE_HEAD(rfcomm_wq);
+
+static void rfcomm_wake(void)
{
- if (!rfcomm_thread)
- return;
- wake_up_process(rfcomm_thread);
+ wake_up_all(&rfcomm_wq);
}
/* ---- RFCOMM FCS computation ---- */
@@ -183,13 +183,13 @@ static inline int __check_fcs(u8 *data,
static void rfcomm_l2state_change(struct sock *sk)
{
BT_DBG("%p state %d", sk, sk->sk_state);
- rfcomm_schedule();
+ rfcomm_wake();
}
static void rfcomm_l2data_ready(struct sock *sk)
{
BT_DBG("%p", sk);
- rfcomm_schedule();
+ rfcomm_wake();
}
static int rfcomm_l2sock_create(struct socket **sock)
@@ -238,7 +238,7 @@ static void rfcomm_session_timeout(unsig
BT_DBG("session %p state %ld", s, s->state);
set_bit(RFCOMM_TIMED_OUT, &s->flags);
- rfcomm_schedule();
+ rfcomm_wake();
}
static void rfcomm_session_set_timer(struct rfcomm_session *s, long timeout)
@@ -264,7 +264,7 @@ static void rfcomm_dlc_timeout(unsigned
set_bit(RFCOMM_TIMED_OUT, &d->flags);
rfcomm_dlc_put(d);
- rfcomm_schedule();
+ rfcomm_wake();
}
static void rfcomm_dlc_set_timer(struct rfcomm_dlc *d, long timeout)
@@ -462,7 +462,7 @@ static int __rfcomm_dlc_close(struct rfc
case BT_CONNECT2:
if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
set_bit(RFCOMM_AUTH_REJECT, &d->flags);
- rfcomm_schedule();
+ rfcomm_wake();
return 0;
}
}
@@ -566,7 +566,7 @@ int rfcomm_dlc_send(struct rfcomm_dlc *d
skb_queue_tail(&d->tx_queue, skb);
if (!test_bit(RFCOMM_TX_THROTTLED, &d->flags))
- rfcomm_schedule();
+ rfcomm_wake();
return len;
}
@@ -581,7 +581,7 @@ void rfcomm_dlc_send_noerror(struct rfco
if (d->state == BT_CONNECTED &&
!test_bit(RFCOMM_TX_THROTTLED, &d->flags))
- rfcomm_schedule();
+ rfcomm_wake();
}
void __rfcomm_dlc_throttle(struct rfcomm_dlc *d)
@@ -592,7 +592,7 @@ void __rfcomm_dlc_throttle(struct rfcomm
d->v24_sig |= RFCOMM_V24_FC;
set_bit(RFCOMM_MSC_PENDING, &d->flags);
}
- rfcomm_schedule();
+ rfcomm_wake();
}
void __rfcomm_dlc_unthrottle(struct rfcomm_dlc *d)
@@ -603,7 +603,7 @@ void __rfcomm_dlc_unthrottle(struct rfco
d->v24_sig &= ~RFCOMM_V24_FC;
set_bit(RFCOMM_MSC_PENDING, &d->flags);
}
- rfcomm_schedule();
+ rfcomm_wake();
}
/*
@@ -624,7 +624,7 @@ int rfcomm_dlc_set_modem_status(struct r
d->v24_sig = v24_sig;
if (!test_and_set_bit(RFCOMM_MSC_PENDING, &d->flags))
- rfcomm_schedule();
+ rfcomm_wake();
return 0;
}
@@ -872,7 +872,7 @@ static int rfcomm_queue_disc(struct rfco
cmd->fcs = __fcs2((u8 *) cmd);
skb_queue_tail(&d->tx_queue, skb);
- rfcomm_schedule();
+ rfcomm_wake();
return 0;
}
@@ -1952,7 +1952,7 @@ static void rfcomm_accept_connection(str
s->mtu = min(l2cap_pi(nsock->sk)->chan->omtu,
l2cap_pi(nsock->sk)->chan->imtu) - 5;
- rfcomm_schedule();
+ rfcomm_wake();
} else
sock_release(nsock);
}
@@ -2086,24 +2086,22 @@ static void rfcomm_kill_listener(void)
static int rfcomm_run(void *unused)
{
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
BT_DBG("");
set_user_nice(current, -10);
rfcomm_add_listener(BDADDR_ANY);
- while (1) {
- set_current_state(TASK_INTERRUPTIBLE);
-
- if (kthread_should_stop())
- break;
+ add_wait_queue(&rfcomm_wq, &wait);
+ while (!kthread_should_stop()) {
/* Process stuff */
rfcomm_process_sessions();
- schedule();
+ wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
}
- __set_current_state(TASK_RUNNING);
+ remove_wait_queue(&rfcomm_wq, &wait);
rfcomm_kill_listener();
@@ -2154,7 +2152,7 @@ static void rfcomm_security_cfm(struct h
set_bit(RFCOMM_AUTH_REJECT, &d->flags);
}
- rfcomm_schedule();
+ rfcomm_wake();
}
static struct hci_cb rfcomm_cb = {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
