Hi Ronald,
> Commit dec2c92880cc5435381d50e3045ef018a762a917 ("Bluetooth: hci_ldisc:
> Use rwlocking to avoid closing proto races") introduced locks in
> hci_ldisc that are held while calling the proto functions. These locks
> are rwlock's, and hence do not allow sleeping while they are held.
> However, the proto functions that hci_bcm registers use mutexes and
> hence need to be able to sleep.
>
> In more detail: hci_uart_tty_receive() and hci_uart_dequeue() both
> acquire the rwlock, after which they call proto->recv() and
> proto->dequeue(), respectively. In the case of hci_bcm these point to
> bcm_recv() and bcm_dequeue(). The latter both acquire the
> bcm_device_lock, which is a mutex, so doing so results in a call to
> might_sleep(). But since we're holding a rwlock in hci_ldisc, that
> results in the following BUG (this for the dequeue case - a similar
> one for the receive case is omitted for brevity):
>
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c
> in_atomic(): 1, irqs_disabled(): 0, pid: 7303, name: kworker/7:3
> INFO: lockdep is turned off.
> CPU: 7 PID: 7303 Comm: kworker/7:3 Tainted: G W OE 4.13.2+ #17
> Hardware name: Apple Inc. MacBookPro13,3/Mac-A5C67F76ED83108C, BIOS MBP133.8
> Workqueue: events hci_uart_write_work [hci_uart]
> Call Trace:
> dump_stack+0x8e/0xd6
> ___might_sleep+0x164/0x250
> __might_sleep+0x4a/0x80
> __mutex_lock+0x59/0xa00
> ? lock_acquire+0xa3/0x1f0
> ? lock_acquire+0xa3/0x1f0
> ? hci_uart_write_work+0xd3/0x160 [hci_uart]
> mutex_lock_nested+0x1b/0x20
> ? mutex_lock_nested+0x1b/0x20
> bcm_dequeue+0x21/0xc0 [hci_uart]
> hci_uart_write_work+0xe6/0x160 [hci_uart]
> process_one_work+0x253/0x6a0
> worker_thread+0x4d/0x3b0
> kthread+0x133/0x150
>
> We can't replace the mutex in hci_bcm, because there are other calls
> there that might sleep. Therefore this replaces the rwlock's in
> hci_ldisc with rw_semaphore's (which allow sleeping). This is a safer
> approach anyway as it reduces the restrictions on the proto callbacks.
> Also, because acquiring write-lock is very rare compared to acquiring
> the read-lock, the percpu variant of rw_semaphore is used.
>
> Lastly, because hci_uart_tx_wakeup() may be called from an IRQ context,
> we can't block (sleep) while trying acquire the read lock there, so we
> use the trylock variant.
>
> Signed-off-by: Ronald Tschalär <ronald@xxxxxxxxxxxxx>
> Cc: Lukas Wunner <lukas@xxxxxxxxx>
> Cc: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> Cc: Gustavo Padovan <gustavo@xxxxxxxxxxx>
> Cc: Johan Hedberg <johan.hedberg@xxxxxxxxx>
> Cc: Dean Jenkins <Dean_Jenkins@xxxxxxxxxx>
> ---
> Changes in v2:
> - Add back locking in hci_uart_tx_wakeup(). Removing the locking
> altogether there was wrong, as nicely pointed out by Dean Jenkins.
>
> drivers/bluetooth/hci_ldisc.c | 38 ++++++++++++++++++++++----------------
> drivers/bluetooth/hci_uart.h | 2 +-
> 2 files changed, 23 insertions(+), 17 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
