[v2,12/14] mac802154: Rename kfree_rcu() to kvfree_rcu_mightsleep()

Message ID 20230315181902.4177819-12-joel@joelfernandes.org
State New
Headers
Series [v2,01/14] drbd: Rename kvfree_rcu() to kvfree_rcu_mightsleep() |

Commit Message

Joel Fernandes March 15, 2023, 6:18 p.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")
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 net/mac802154/scan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

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

On 15.03.23 19:18, 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")
> Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
>   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);
>   

I just saw that there is a v2 of this patch. My ACK still stands as for v1.


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

regards
Stefan Schmidt
  
Joel Fernandes March 16, 2023, 6:06 p.m. UTC | #2
On Thu, Mar 16, 2023 at 12:41 PM Stefan Schmidt
<stefan@datenfreihafen.org> wrote:
>
> Hello.
>
> On 15.03.23 19:18, 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")
> > Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> >   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);
> >
>
> I just saw that there is a v2 of this patch. My ACK still stands as for v1.
>
>
> Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>

Thanks! Applied the ack and will be taking it via the RCU tree as we discussed.

 - Joel
  

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);