[PATCH 1/2] rpmb: Remove usage of the deprecated ida_simple_xx() API

[PATCH 1/2] rpmb: Remove usage of the deprecated ida_simple_xx() API

[Christophe JAILLET]

ida_alloc() and ida_free() should be preferred to the deprecated
ida_simple_get() and ida_simple_remove().

This is less verbose.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
The ida_simple_get()/ida_simple_remove() API was close to be removed (see
[1]). A usage has been re-introduced with this new driver :(

[1]: https://lore.kernel.org/all/cover.1722853349.git.christophe.jaillet@wanadoo.fr/
---
 drivers/misc/rpmb-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
index bc68cde1a8bf..ad1b5c1a37fa 100644
--- a/drivers/misc/rpmb-core.c
+++ b/drivers/misc/rpmb-core.c
@@ -64,7 +64,7 @@ static void rpmb_dev_release(struct device *dev)
 	struct rpmb_dev *rdev = to_rpmb_dev(dev);
 
 	mutex_lock(&rpmb_mutex);
-	ida_simple_remove(&rpmb_ida, rdev->id);
+	ida_free(&rpmb_ida, rdev->id);
 	mutex_unlock(&rpmb_mutex);
 	kfree(rdev->descr.dev_id);
 	kfree(rdev);
@@ -176,7 +176,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
 	}
 
 	mutex_lock(&rpmb_mutex);
-	ret = ida_simple_get(&rpmb_ida, 0, 0, GFP_KERNEL);
+	ret = ida_alloc(&rpmb_ida, GFP_KERNEL);
 	mutex_unlock(&rpmb_mutex);
 	if (ret < 0)
 		goto err_free_dev_id;
-- 
2.46.2

[PATCH 2/2] rpmb: Remove some useless locking

[Christophe JAILLET]

There is no need for explicit locking when using the ida API, as stated in
the doc related to ida_alloc_range() / ida_free().

So remove rpmb_mutex.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
See:
https://elixir.bootlin.com/linux/v6.11.2/source/lib/idr.c#L375
https://elixir.bootlin.com/linux/v6.11.2/source/lib/idr.c#L484
---
 drivers/misc/rpmb-core.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
index ad1b5c1a37fa..2d653926cdbb 100644
--- a/drivers/misc/rpmb-core.c
+++ b/drivers/misc/rpmb-core.c
@@ -13,7 +13,6 @@
 #include <linux/slab.h>
 
 static DEFINE_IDA(rpmb_ida);
-static DEFINE_MUTEX(rpmb_mutex);
 
 /**
  * rpmb_dev_get() - increase rpmb device ref counter
@@ -63,9 +62,7 @@ static void rpmb_dev_release(struct device *dev)
 {
 	struct rpmb_dev *rdev = to_rpmb_dev(dev);
 
-	mutex_lock(&rpmb_mutex);
 	ida_free(&rpmb_ida, rdev->id);
-	mutex_unlock(&rpmb_mutex);
 	kfree(rdev->descr.dev_id);
 	kfree(rdev);
 }
@@ -175,9 +172,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
 		goto err_free_rdev;
 	}
 
-	mutex_lock(&rpmb_mutex);
 	ret = ida_alloc(&rpmb_ida, GFP_KERNEL);
-	mutex_unlock(&rpmb_mutex);
 	if (ret < 0)
 		goto err_free_dev_id;
 	rdev->id = ret;
-- 
2.46.2

Re: [PATCH 2/2] rpmb: Remove some useless locking

[Jens Wiklander]

On Wed, Oct 9, 2024 at 10:53 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> There is no need for explicit locking when using the ida API, as stated in
> the doc related to ida_alloc_range() / ida_free().
>
> So remove rpmb_mutex.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> See:
> https://elixir.bootlin.com/linux/v6.11.2/source/lib/idr.c#L375
> https://elixir.bootlin.com/linux/v6.11.2/source/lib/idr.c#L484
> ---
>  drivers/misc/rpmb-core.c | 5 -----
>  1 file changed, 5 deletions(-)

I'm picking up this for v6.13.

Thanks,
Jens

>
> diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
> index ad1b5c1a37fa..2d653926cdbb 100644
> --- a/drivers/misc/rpmb-core.c
> +++ b/drivers/misc/rpmb-core.c
> @@ -13,7 +13,6 @@
>  #include <linux/slab.h>
>
>  static DEFINE_IDA(rpmb_ida);
> -static DEFINE_MUTEX(rpmb_mutex);
>
>  /**
>   * rpmb_dev_get() - increase rpmb device ref counter
> @@ -63,9 +62,7 @@ static void rpmb_dev_release(struct device *dev)
>  {
>         struct rpmb_dev *rdev = to_rpmb_dev(dev);
>
> -       mutex_lock(&rpmb_mutex);
>         ida_free(&rpmb_ida, rdev->id);
> -       mutex_unlock(&rpmb_mutex);
>         kfree(rdev->descr.dev_id);
>         kfree(rdev);
>  }
> @@ -175,9 +172,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
>                 goto err_free_rdev;
>         }
>
> -       mutex_lock(&rpmb_mutex);
>         ret = ida_alloc(&rpmb_ida, GFP_KERNEL);
> -       mutex_unlock(&rpmb_mutex);
>         if (ret < 0)
>                 goto err_free_dev_id;
>         rdev->id = ret;
> --
> 2.46.2
>

Re: [PATCH 1/2] rpmb: Remove usage of the deprecated ida_simple_xx() API

[Jens Wiklander]

On Wed, Oct 9, 2024 at 10:53 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> ida_alloc() and ida_free() should be preferred to the deprecated
> ida_simple_get() and ida_simple_remove().
>
> This is less verbose.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> The ida_simple_get()/ida_simple_remove() API was close to be removed (see
> [1]). A usage has been re-introduced with this new driver :(
>
> [1]: https://lore.kernel.org/all/cover.1722853349.git.christophe.jaillet@wanadoo.fr/

I'm picking up this for v6.13. I guess your patch set now depends on
this patch. I can ack the patch instead and let you include it in your
patch set if it's more convenient. Please let me know what you prefer
to avoid potential conflicts.

Thanks,
Jens

> ---
>  drivers/misc/rpmb-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
> index bc68cde1a8bf..ad1b5c1a37fa 100644
> --- a/drivers/misc/rpmb-core.c
> +++ b/drivers/misc/rpmb-core.c
> @@ -64,7 +64,7 @@ static void rpmb_dev_release(struct device *dev)
>         struct rpmb_dev *rdev = to_rpmb_dev(dev);
>
>         mutex_lock(&rpmb_mutex);
> -       ida_simple_remove(&rpmb_ida, rdev->id);
> +       ida_free(&rpmb_ida, rdev->id);
>         mutex_unlock(&rpmb_mutex);
>         kfree(rdev->descr.dev_id);
>         kfree(rdev);
> @@ -176,7 +176,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
>         }
>
>         mutex_lock(&rpmb_mutex);
> -       ret = ida_simple_get(&rpmb_ida, 0, 0, GFP_KERNEL);
> +       ret = ida_alloc(&rpmb_ida, GFP_KERNEL);
>         mutex_unlock(&rpmb_mutex);
>         if (ret < 0)
>                 goto err_free_dev_id;
> --
> 2.46.2
>

Re: [PATCH 1/2] rpmb: Remove usage of the deprecated ida_simple_xx() API

[Christophe JAILLET]

Le 14/10/2024 à 12:38, Jens Wiklander a écrit :
> On Wed, Oct 9, 2024 at 10:53 PM Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:
>>
>> ida_alloc() and ida_free() should be preferred to the deprecated
>> ida_simple_get() and ida_simple_remove().
>>
>> This is less verbose.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> The ida_simple_get()/ida_simple_remove() API was close to be removed (see
>> [1]). A usage has been re-introduced with this new driver :(
>>
>> [1]: https://lore.kernel.org/all/cover.1722853349.git.christophe.jaillet@wanadoo.fr/
> 
> I'm picking up this for v6.13. I guess your patch set now depends on
> this patch. I can ack the patch instead and let you include it in your
> patch set if it's more convenient. Please let me know what you prefer
> to avoid potential conflicts.

Greg has already picked both patches in char-misc-testing. [1], [2].

Let it flow from his trees.

I'll wait the next cycle to resend my serie [3] or char-misc could take 
it as well. Both solution are fine with me.


Greg, if you prefer the later, I can resend the serie if more convenient 
to you.

CJ


[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-testing&id=dfc881abca4247dcf453ce206f05fe09b51be158

[2]: 
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-testing&id=3b0889f95789aa90b0f1a6921d5d6b151f2e53ae

[3]: 
https://lore.kernel.org/all/cover.1722853349.git.christophe.jaillet@wanadoo.fr/

> 
> Thanks,
> Jens
> 
>> ---
>>   drivers/misc/rpmb-core.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
>> index bc68cde1a8bf..ad1b5c1a37fa 100644
>> --- a/drivers/misc/rpmb-core.c
>> +++ b/drivers/misc/rpmb-core.c
>> @@ -64,7 +64,7 @@ static void rpmb_dev_release(struct device *dev)
>>          struct rpmb_dev *rdev = to_rpmb_dev(dev);
>>
>>          mutex_lock(&rpmb_mutex);
>> -       ida_simple_remove(&rpmb_ida, rdev->id);
>> +       ida_free(&rpmb_ida, rdev->id);
>>          mutex_unlock(&rpmb_mutex);
>>          kfree(rdev->descr.dev_id);
>>          kfree(rdev);
>> @@ -176,7 +176,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
>>          }
>>
>>          mutex_lock(&rpmb_mutex);
>> -       ret = ida_simple_get(&rpmb_ida, 0, 0, GFP_KERNEL);
>> +       ret = ida_alloc(&rpmb_ida, GFP_KERNEL);
>>          mutex_unlock(&rpmb_mutex);
>>          if (ret < 0)
>>                  goto err_free_dev_id;
>> --
>> 2.46.2
>>
> 
> 

Re: [PATCH 1/2] rpmb: Remove usage of the deprecated ida_simple_xx() API

[Greg Kroah-Hartman]

On Mon, Oct 14, 2024 at 06:53:56PM +0200, Christophe JAILLET wrote:
> Le 14/10/2024 à 12:38, Jens Wiklander a écrit :
> > On Wed, Oct 9, 2024 at 10:53 PM Christophe JAILLET
> > <christophe.jaillet@wanadoo.fr> wrote:
> > > 
> > > ida_alloc() and ida_free() should be preferred to the deprecated
> > > ida_simple_get() and ida_simple_remove().
> > > 
> > > This is less verbose.
> > > 
> > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > > ---
> > > The ida_simple_get()/ida_simple_remove() API was close to be removed (see
> > > [1]). A usage has been re-introduced with this new driver :(
> > > 
> > > [1]: https://lore.kernel.org/all/cover.1722853349.git.christophe.jaillet@wanadoo.fr/
> > 
> > I'm picking up this for v6.13. I guess your patch set now depends on
> > this patch. I can ack the patch instead and let you include it in your
> > patch set if it's more convenient. Please let me know what you prefer
> > to avoid potential conflicts.
> 
> Greg has already picked both patches in char-misc-testing. [1], [2].
> 
> Let it flow from his trees.
> 
> I'll wait the next cycle to resend my serie [3] or char-misc could take it
> as well. Both solution are fine with me.
> 
> 
> Greg, if you prefer the later, I can resend the serie if more convenient to
> you.
> 
> CJ
> 
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-testing&id=dfc881abca4247dcf453ce206f05fe09b51be158
> 
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-testing&id=3b0889f95789aa90b0f1a6921d5d6b151f2e53ae
> 
> [3]: https://lore.kernel.org/all/cover.1722853349.git.christophe.jaillet@wanadoo.fr/

If this is the last users that got taken away in my tree, I'll be glad
to take the removal series as well, so please send it on!

thanks,

greg k-h