rfcomm/bluetooth: avoid disabling interrupts on RT

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.

index b4eaf21360ef..f894dc14bad0 100644
@@ -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

On 2020-04-23 16:31:55 [-0300], Luis Claudio R. Goncalves wrote:
…

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

Thanks, Sebastian! I built a kernel with commit fad003b6c8e3d reverted and the lockdep complaints are gone.

Luis