diff mbox series

mac802154: Rename kfree_rcu() to kvfree_rcu_mightsleep()

Message ID 20230310013144.970964-1-joel@joelfernandes.org (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series mac802154: Rename kfree_rcu() to kvfree_rcu_mightsleep() | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 20 this patch: 20
netdev/cc_maintainers fail 1 blamed authors not CCed: david.girault@qorvo.com; 1 maintainers not CCed: david.girault@qorvo.com
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 20 this patch: 20
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Joel Fernandes March 10, 2023, 1:31 a.m. UTC
The k[v]free_rcu() macro's single-argument form is deprecated.
Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
is to avoid accidental use of the single-argument forms, which can
introduce functionality bugs in atomic contexts and latency bugs in
non-atomic contexts.

The callers are holding a mutex so the context allows blocking. Hence
using the API with a single argument will be fine, but use its new name.

There is no functionality change with this patch.

Fixes: 57588c71177f ("mac802154: Handle passive scanning")
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
Please Ack the patch but we can carry it through the RCU tree as well if
needed, as it is not a bug per-se and we are not dropping the old API before
the next release.

 net/mac802154/scan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Stefan Schmidt March 16, 2023, 4:36 p.m. UTC | #1
Hello Joel.

On 10.03.23 02:31, Joel Fernandes (Google) wrote:
> The k[v]free_rcu() macro's single-argument form is deprecated.
> Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
> is to avoid accidental use of the single-argument forms, which can
> introduce functionality bugs in atomic contexts and latency bugs in
> non-atomic contexts.
> 
> The callers are holding a mutex so the context allows blocking. Hence
> using the API with a single argument will be fine, but use its new name.
> 
> There is no functionality change with this patch.
> 
> Fixes: 57588c71177f ("mac802154: Handle passive scanning")
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> Please Ack the patch but we can carry it through the RCU tree as well if
> needed, as it is not a bug per-se and we are not dropping the old API before
> the next release.

The "but we can carry it" part throws me off here. Not sure if you want 
this through the RCU tree (I suppose). In that case see my ack below.

If you want me to take it through my wpan tree instead let me know.

>   net/mac802154/scan.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
> index 9b0933a185eb..5c191bedd72c 100644
> --- a/net/mac802154/scan.c
> +++ b/net/mac802154/scan.c
> @@ -52,7 +52,7 @@ static int mac802154_scan_cleanup_locked(struct ieee802154_local *local,
>   	request = rcu_replace_pointer(local->scan_req, NULL, 1);
>   	if (!request)
>   		return 0;
> -	kfree_rcu(request);
> +	kvfree_rcu_mightsleep(request);
>   
>   	/* Advertize first, while we know the devices cannot be removed */
>   	if (aborted)
> @@ -403,7 +403,7 @@ int mac802154_stop_beacons_locked(struct ieee802154_local *local,
>   	request = rcu_replace_pointer(local->beacon_req, NULL, 1);
>   	if (!request)
>   		return 0;
> -	kfree_rcu(request);
> +	kvfree_rcu_mightsleep(request);
>   
>   	nl802154_beaconing_done(wpan_dev);
>   


Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>

regards
Stefan Schmidt
Joel Fernandes March 16, 2023, 5:58 p.m. UTC | #2
> On Mar 16, 2023, at 12:36 PM, Stefan Schmidt <stefan@datenfreihafen.org> wrote:
> 
> Hello Joel.
> 
>> On 10.03.23 02:31, Joel Fernandes (Google) wrote:
>> The k[v]free_rcu() macro's single-argument form is deprecated.
>> Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
>> is to avoid accidental use of the single-argument forms, which can
>> introduce functionality bugs in atomic contexts and latency bugs in
>> non-atomic contexts.
>> The callers are holding a mutex so the context allows blocking. Hence
>> using the API with a single argument will be fine, but use its new name.
>> There is no functionality change with this patch.
>> Fixes: 57588c71177f ("mac802154: Handle passive scanning")
>> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>> ---
>> Please Ack the patch but we can carry it through the RCU tree as well if
>> needed, as it is not a bug per-se and we are not dropping the old API before
>> the next release.
> 
> The "but we can carry it" part throws me off here. Not sure if you want this through the RCU tree (I suppose). In that case see my ack below.
> 
> If you want me to take it through my wpan tree instead let me know.

We will take this with your Ack below, thank you!

 - Joel


>>  net/mac802154/scan.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
>> index 9b0933a185eb..5c191bedd72c 100644
>> --- a/net/mac802154/scan.c
>> +++ b/net/mac802154/scan.c
>> @@ -52,7 +52,7 @@ static int mac802154_scan_cleanup_locked(struct ieee802154_local *local,
>>      request = rcu_replace_pointer(local->scan_req, NULL, 1);
>>      if (!request)
>>          return 0;
>> -    kfree_rcu(request);
>> +    kvfree_rcu_mightsleep(request);
>>        /* Advertize first, while we know the devices cannot be removed */
>>      if (aborted)
>> @@ -403,7 +403,7 @@ int mac802154_stop_beacons_locked(struct ieee802154_local *local,
>>      request = rcu_replace_pointer(local->beacon_req, NULL, 1);
>>      if (!request)
>>          return 0;
>> -    kfree_rcu(request);
>> +    kvfree_rcu_mightsleep(request);
>>        nl802154_beaconing_done(wpan_dev);
>>  
> 
> 
> Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
> 
> regards
> Stefan Schmidt
diff mbox series

Patch

diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index 9b0933a185eb..5c191bedd72c 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -52,7 +52,7 @@  static int mac802154_scan_cleanup_locked(struct ieee802154_local *local,
 	request = rcu_replace_pointer(local->scan_req, NULL, 1);
 	if (!request)
 		return 0;
-	kfree_rcu(request);
+	kvfree_rcu_mightsleep(request);
 
 	/* Advertize first, while we know the devices cannot be removed */
 	if (aborted)
@@ -403,7 +403,7 @@  int mac802154_stop_beacons_locked(struct ieee802154_local *local,
 	request = rcu_replace_pointer(local->beacon_req, NULL, 1);
 	if (!request)
 		return 0;
-	kfree_rcu(request);
+	kvfree_rcu_mightsleep(request);
 
 	nl802154_beaconing_done(wpan_dev);