LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Marcel Holtmann <firstname.lastname@example.org>
To: Desmond Cheong Zhi Xi <email@example.com>
Cc: Johan Hedberg <firstname.lastname@example.org>,
Luiz Augusto von Dentz <email@example.com>,
"David S. Miller" <firstname.lastname@example.org>,
Jakub Kicinski <email@example.com>,
Matthieu Baerts <firstname.lastname@example.org>,
Stefan Schmidt <email@example.com>,
"open list:NETWORKING [GENERAL]" <firstname.lastname@example.org>,
open list <email@example.com>,
Greg Kroah-Hartman <firstname.lastname@example.org>,
Subject: Re: [PATCH v3 2/2] Bluetooth: fix inconsistent lock state in rfcomm_connect_ind
Date: Fri, 30 Jul 2021 15:40:19 +0200 [thread overview]
Message-ID: <B35BD760-E00E-43ED-85A1-775197FB3ED1@holtmann.org> (raw)
>>> Commit fad003b6c8e3d ("Bluetooth: Fix inconsistent lock state with
>>> RFCOMM") fixed a lockdep warning due to sk->sk_lock.slock being
>>> acquired without disabling softirq while the lock is also used in
>>> softirq context. This was done by disabling interrupts before calling
>>> bh_lock_sock in rfcomm_sk_state_change.
>>> Later, this was changed in commit e6da0edc24ee ("Bluetooth: Acquire
>>> sk_lock.slock without disabling interrupts") to disable softirqs
>>> However, there is another instance of sk->sk_lock.slock being acquired
>>> without disabling softirq in rfcomm_connect_ind. This patch fixes this
>>> by disabling local bh before the call to bh_lock_sock.
>> back in the days, the packet processing was done in a tasklet, but these days it is done in a workqueue. So shouldn’t this be just converted into a lock_sock(). Am I missing something?
> Thanks for the info. I think you're right, I just didn't understand very much when I wrote this patch.
> If I'm understanding correctly, it seems that both the bh_lock_sock in rfcomm_connect_ind, and spin_lock_bh in rfcomm_sk_state_change need to be changed to lock_sock, otherwise they don't provide any synchronization with other functions in RFCOMM that use lock_sock.
> If that sounds correct I can prepare the patch for that.
please do so and re-run the tests. Thanks.
prev parent reply other threads:[~2021-07-30 13:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-21 9:38 [PATCH v3 0/2] Bluetooth: fix inconsistent lock states Desmond Cheong Zhi Xi
2021-07-21 9:38 ` [PATCH v3 1/2] Bluetooth: fix inconsistent lock state in SCO Desmond Cheong Zhi Xi
2021-07-27 0:30 ` Luiz Augusto von Dentz
2021-07-27 5:13 ` Desmond Cheong Zhi Xi
2021-07-21 9:38 ` [PATCH v3 2/2] Bluetooth: fix inconsistent lock state in rfcomm_connect_ind Desmond Cheong Zhi Xi
2021-07-29 19:53 ` Marcel Holtmann
2021-07-30 9:06 ` Desmond Cheong Zhi Xi
2021-07-30 13:40 ` Marcel Holtmann [this message]
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [PATCH v3 2/2] Bluetooth: fix inconsistent lock state in rfcomm_connect_ind' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).