[PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT

[PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT

[Luis Claudio R. Goncalves]

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

Re: [PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT

[Sebastian Andrzej Siewior]

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

Re: [PATCH v5.6.4-rt3] rfcomm/bluetooth: avoid disabling interrupts on RT

[Luis Claudio R. Goncalves]

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