diff mbox series

[rtw-next] wifi: rtw89: sar: drop assertion from rtw89_sar_set_src()

Message ID 20250603152642.185672-1-pchelkin@ispras.ru
State New
Headers show
Series [rtw-next] wifi: rtw89: sar: drop assertion from rtw89_sar_set_src() | expand

Commit Message

Fedor Pchelkin June 3, 2025, 3:26 p.m. UTC
rtw89_sar_set_src() may be called at driver early init phase when
applying SAR configuration via ACPI. wiphy lock is not held there.

The only other callsite of the macro triggered by netlink handler
originates from rtw89_ops_set_sar_specs() and does already have the wiphy
lock assertion - and it is actually needed there.

Remove the assertion here in rtw89_sar_set_src() as it may lead to false
alarms now.

Found by Linux Verification Center (linuxtesting.org).

Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---

Urgh, this one wasn't caught as my system doesn't have any SAR available
from ACPI. But it would be falsely triggered, too. If I saw it earlier,
I'd better prepared this as a followup patch in a series though..

 drivers/net/wireless/realtek/rtw89/sar.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Ping-Ke Shih June 4, 2025, 1:28 a.m. UTC | #1
Fedor Pchelkin <pchelkin@ispras.ru> wrote:
> Urgh, this one wasn't caught as my system doesn't have any SAR available
> from ACPI. But it would be falsely triggered, too. If I saw it earlier,
> I'd better prepared this as a followup patch in a series though..
> 

Good catch. 

There are two consumers. One is rtw89_apply_sar_acpi() which should not
assert wiphy_lock, but the other rtw89_apply_sar_common() can be. As I know,
the assertion is added for the latter one initially.

Another way is to assert the lock under condition of 
   test_bit(RTW89_FLAG_PROBE_DONE, rtwdev->flags)


>  drivers/net/wireless/realtek/rtw89/sar.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/sar.c b/drivers/net/wireless/realtek/rtw89/sar.c
> index 33a4b5c23fe7..3f57881b74e6 100644
> --- a/drivers/net/wireless/realtek/rtw89/sar.c
> +++ b/drivers/net/wireless/realtek/rtw89/sar.c
> @@ -199,7 +199,6 @@ struct rtw89_sar_handler rtw89_sar_handlers[RTW89_SAR_SOURCE_NR] = {
>                 typeof(_dev) _d = (_dev);                               \
>                 BUILD_BUG_ON(!rtw89_sar_handlers[_s].descr_sar_source); \
>                 BUILD_BUG_ON(!rtw89_sar_handlers[_s].query_sar_config); \
> -               lockdep_assert_wiphy(_d->hw->wiphy);                    \
>                 _d->sar._cfg_name = *(_cfg_data);                       \
>                 _d->sar.src = _s;                                       \
>         } while (0)
> --
> 2.49.0
>
Fedor Pchelkin June 4, 2025, 4:06 p.m. UTC | #2
On Wed, 04. Jun 01:28, Ping-Ke Shih wrote:
> Fedor Pchelkin <pchelkin@ispras.ru> wrote:
> > Urgh, this one wasn't caught as my system doesn't have any SAR available
> > from ACPI. But it would be falsely triggered, too. If I saw it earlier,
> > I'd better prepared this as a followup patch in a series though..
> > 
> 
> Good catch. 
> 
> There are two consumers. One is rtw89_apply_sar_acpi() which should not
> assert wiphy_lock, but the other rtw89_apply_sar_common() can be. As I know,
> the assertion is added for the latter one initially.
> 
> Another way is to assert the lock under condition of 
>    test_bit(RTW89_FLAG_PROBE_DONE, rtwdev->flags)
> 

Ok, thanks! So I'll fold this reworked patch and the other one [1] into
a series and send them out.

[1]: https://lore.kernel.org/linux-wireless/20250603144614.175003-1-pchelkin@ispras.ru/
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw89/sar.c b/drivers/net/wireless/realtek/rtw89/sar.c
index 33a4b5c23fe7..3f57881b74e6 100644
--- a/drivers/net/wireless/realtek/rtw89/sar.c
+++ b/drivers/net/wireless/realtek/rtw89/sar.c
@@ -199,7 +199,6 @@  struct rtw89_sar_handler rtw89_sar_handlers[RTW89_SAR_SOURCE_NR] = {
 		typeof(_dev) _d = (_dev);				\
 		BUILD_BUG_ON(!rtw89_sar_handlers[_s].descr_sar_source);	\
 		BUILD_BUG_ON(!rtw89_sar_handlers[_s].query_sar_config);	\
-		lockdep_assert_wiphy(_d->hw->wiphy);			\
 		_d->sar._cfg_name = *(_cfg_data);			\
 		_d->sar.src = _s;					\
 	} while (0)