* [PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT
@ 2020-04-23 19:31 Luis Claudio R. Goncalves
2020-05-27 9:48 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 3+ messages in thread
From: Luis Claudio R. Goncalves @ 2020-04-23 19:31 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, linux-rt-users
The current code in rfcomm_sk_state_change() calls:
local_irq_save(flags);
bh_lock_sock(sk);
As bh_lock_sock translates into a spin_lock() operation, the following BUG is
triggered on PREEMPT_RT:
[ 277.573392] BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:973
[ 277.573399] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 2234, name: krfcommd
[ 277.573401] INFO: lockdep is turned off.
[ 277.573402] irq event stamp: 0
[ 277.573403] hardirqs last enabled at (0): [<0000000000000000>] 0x0
[ 277.573407] hardirqs last disabled at (0): [<ffffffff910ed9df>] copy_process+0x7cf/0x2100
[ 277.573412] softirqs last enabled at (0): [<ffffffff910ed9df>] copy_process+0x7cf/0x2100
[ 277.573414] softirqs last disabled at (0): [<0000000000000000>] 0x0
[ 277.573416] CPU: 1 PID: 2234 Comm: krfcommd Tainted: G W 5.6.4.lockdep-rt3 #3
[ 277.573419] Hardware name: Hewlett-Packard p7-1512/2ADA, BIOS 8.15 02/05/2013
[ 277.573420] Call Trace:
[ 277.573427] dump_stack+0x8f/0xd0
[ 277.573434] ___might_sleep.cold+0xf5/0x109
[ 277.573441] rt_spin_lock+0x88/0xc0
[ 277.573447] ? rfcomm_sk_state_change+0x55/0x190 [rfcomm]
[ 277.573455] rfcomm_sk_state_change+0x55/0x190 [rfcomm]
[ 277.573462] rfcomm_run+0x1340/0x18f0 [rfcomm]
[ 277.573471] ? do_wait_intr_irq+0xc0/0xc0
[ 277.573478] ? _raw_spin_unlock_irqrestore+0x41/0x90
[ 277.573484] kthread+0x106/0x140
[ 277.573488] ? rfcomm_check_accept+0x90/0x90 [rfcomm]
[ 277.573492] ? kthread_park+0x90/0x90
[ 277.573496] ret_from_fork+0x3a/0x50
Pairing a bluetooth device is enough to trigger the BUG.
Replacing local_irq_save()/bh_lock_sock() by spin_lock_bh() keeps the semantics
and does the right thing under PREEMPT_RT.
Signed-off-by: Luis Claudio R. Goncalves <lgoncalv@redhat.com>
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index b4eaf21360ef..f894dc14bad0 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -64,15 +64,13 @@ static void rfcomm_sk_data_ready(struct rfcomm_dlc *d, struct sk_buff *skb)
static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
{
struct sock *sk = d->owner, *parent;
- unsigned long flags;
if (!sk)
return;
BT_DBG("dlc %p state %ld err %d", d, d->state, err);
- local_irq_save(flags);
- bh_lock_sock(sk);
+ spin_lock_bh(&sk->sk_lock.slock);
if (err)
sk->sk_err = err;
@@ -93,8 +91,7 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
sk->sk_state_change(sk);
}
- bh_unlock_sock(sk);
- local_irq_restore(flags);
+ spin_unlock_bh(&sk->sk_lock.slock);
if (parent && sock_flag(sk, SOCK_ZAPPED)) {
/* We have to drop DLC lock here, otherwise
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT
2020-04-23 19:31 [PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT Luis Claudio R. Goncalves
@ 2020-05-27 9:48 ` Sebastian Andrzej Siewior
2020-05-27 17:40 ` Luis Claudio R. Goncalves
0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-05-27 9:48 UTC (permalink / raw)
To: Luis Claudio R. Goncalves; +Cc: linux-rt-users
On 2020-04-23 16:31:55 [-0300], Luis Claudio R. Goncalves wrote:
…
> Replacing local_irq_save()/bh_lock_sock() by spin_lock_bh() keeps the semantics
> and does the right thing under PREEMPT_RT.
Nope. The irq-off was introduced in commit
fad003b6c8e3d ("Bluetooth: Fix inconsistent lock state with RFCOMM")
It doesn't look right. Could you please revert that commit and check if
lockdep still complains?
Sebastian
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT
2020-05-27 9:48 ` Sebastian Andrzej Siewior
@ 2020-05-27 17:40 ` Luis Claudio R. Goncalves
0 siblings, 0 replies; 3+ messages in thread
From: Luis Claudio R. Goncalves @ 2020-05-27 17:40 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: linux-rt-users
On Wed, May 27, 2020 at 11:48:58AM +0200, Sebastian Andrzej Siewior wrote:
> On 2020-04-23 16:31:55 [-0300], Luis Claudio R. Goncalves wrote:
> …
> > Replacing local_irq_save()/bh_lock_sock() by spin_lock_bh() keeps the semantics
> > and does the right thing under PREEMPT_RT.
>
> Nope. The irq-off was introduced in commit
> fad003b6c8e3d ("Bluetooth: Fix inconsistent lock state with RFCOMM")
>
> It doesn't look right. Could you please revert that commit and check if
> lockdep still complains?
Thanks, Sebastian! I built a kernel with commit fad003b6c8e3d reverted and
the lockdep complaints are gone.
Luis
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-27 17:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-23 19:31 [PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT Luis Claudio R. Goncalves
2020-05-27 9:48 ` Sebastian Andrzej Siewior
2020-05-27 17:40 ` Luis Claudio R. Goncalves
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.