From: Marcel Holtmann <marcel@holtmann.org>
To: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Cc: Johan Hedberg <johan.hedberg@gmail.com>,
Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Matthieu Baerts <matthieu.baerts@tessares.net>,
Stefan Schmidt <stefan@datenfreihafen.org>,
linux-bluetooth <linux-bluetooth@vger.kernel.org>,
"open list:NETWORKING [GENERAL]" <netdev@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>,
skhan@linuxfoundation.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [PATCH v3 2/2] Bluetooth: fix inconsistent lock state in rfcomm_connect_ind
Date: Thu, 29 Jul 2021 21:53:19 +0200 [thread overview]
Message-ID: <06E57598-5723-459D-9CE3-4DD8D3145D86@holtmann.org> (raw)
In-Reply-To: <20210721093832.78081-3-desmondcheongzx@gmail.com>
Hi Desmond,
> 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
> only.
>
> 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?
Regards
Marcel
WARNING: multiple messages have this Message-ID (diff)
From: Marcel Holtmann <marcel@holtmann.org>
To: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Cc: Johan Hedberg <johan.hedberg@gmail.com>,
"open list:NETWORKING \[GENERAL\]" <netdev@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>,
linux-bluetooth <linux-bluetooth@vger.kernel.org>,
Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
Matthieu Baerts <matthieu.baerts@tessares.net>,
Jakub Kicinski <kuba@kernel.org>,
linux-kernel-mentees@lists.linuxfoundation.org,
"David S. Miller" <davem@davemloft.net>,
Stefan Schmidt <stefan@datenfreihafen.org>
Subject: Re: [PATCH v3 2/2] Bluetooth: fix inconsistent lock state in rfcomm_connect_ind
Date: Thu, 29 Jul 2021 21:53:19 +0200 [thread overview]
Message-ID: <06E57598-5723-459D-9CE3-4DD8D3145D86@holtmann.org> (raw)
In-Reply-To: <20210721093832.78081-3-desmondcheongzx@gmail.com>
Hi Desmond,
> 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
> only.
>
> 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?
Regards
Marcel
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2021-07-29 19:53 UTC|newest]
Thread overview: 17+ 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 ` 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-21 9:38 ` Desmond Cheong Zhi Xi
2021-07-21 11:09 ` Bluetooth: fix inconsistent lock states bluez.test.bot
2021-07-27 0:30 ` [PATCH v3 1/2] Bluetooth: fix inconsistent lock state in SCO Luiz Augusto von Dentz
2021-07-27 0:30 ` Luiz Augusto von Dentz
2021-07-27 5:13 ` Desmond Cheong Zhi Xi
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-21 9:38 ` Desmond Cheong Zhi Xi
2021-07-29 19:53 ` Marcel Holtmann [this message]
2021-07-29 19:53 ` Marcel Holtmann
2021-07-30 9:06 ` Desmond Cheong Zhi Xi
2021-07-30 9:06 ` Desmond Cheong Zhi Xi
2021-07-30 13:40 ` Marcel Holtmann
2021-07-30 13:40 ` Marcel Holtmann
Reply instructions:
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=06E57598-5723-459D-9CE3-4DD8D3145D86@holtmann.org \
--to=marcel@holtmann.org \
--cc=davem@davemloft.net \
--cc=desmondcheongzx@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=johan.hedberg@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luiz.dentz@gmail.com \
--cc=matthieu.baerts@tessares.net \
--cc=netdev@vger.kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=stefan@datenfreihafen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.