2023-12-19 06:28:56

by Christophe JAILLET

Subject: [PATCH] dmaengine: idxd: Remove usage of the deprecated ida_simple_xx() API

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

This is less verbose.

Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_range() is inclusive. So change this change allows one more
device.

MINORMASK is ((1U << MINORBITS) - 1), so allowing MINORMASK as a maximum
value makes sense. It is also consistent with other "ida_.*MINORMASK" and
"ida_*MINOR()" usages.

Signed-off-by: Christophe JAILLET <[email protected]>
---
Compile tested only, review with care for the upper bound change.
---
drivers/dma/idxd/cdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
index 0423655f5a88..b00926abc69a 100644
--- a/drivers/dma/idxd/cdev.c
+++ b/drivers/dma/idxd/cdev.c
@@ -165,7 +165,7 @@ static void idxd_cdev_dev_release(struct device *dev)
struct idxd_wq *wq = idxd_cdev->wq;

cdev_ctx = &ictx[wq->idxd->data->type];
- ida_simple_remove(&cdev_ctx->minor_ida, idxd_cdev->minor);
+ ida_free(&cdev_ctx->minor_ida, idxd_cdev->minor);
kfree(idxd_cdev);
}

@@ -463,7 +463,7 @@ int idxd_wq_add_cdev(struct idxd_wq *wq)
cdev = &idxd_cdev->cdev;
dev = cdev_dev(idxd_cdev);
cdev_ctx = &ictx[wq->idxd->data->type];
- minor = ida_simple_get(&cdev_ctx->minor_ida, 0, MINORMASK, GFP_KERNEL);
+ minor = ida_alloc_max(&cdev_ctx->minor_ida, MINORMASK, GFP_KERNEL);
if (minor < 0) {
kfree(idxd_cdev);
return minor;
--
2.34.1

2023-12-19 19:09:35

by Fenghua Yu

Subject: Re: [PATCH] dmaengine: idxd: Remove usage of the deprecated ida_simple_xx() API

On 12/18/23 22:28, Christophe JAILLET wrote:
> ida_alloc() and ida_free() should be preferred to the deprecated
> ida_simple_get() and ida_simple_remove().
>
> This is less verbose.
>
> Note that the upper limit of ida_simple_get() is exclusive, but the one of
> ida_alloc_range() is inclusive. So change this change allows one more

s/change this change/this change/

> device.
>
> MINORMASK is ((1U << MINORBITS) - 1), so allowing MINORMASK as a maximum
Please remove the tab in "is ".

> value makes sense. It is also consistent with other "ida_.*MINORMASK" and
> "ida_*MINOR()" usages.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> Compile tested only, review with care for the upper bound change.

Tested on hw.

> ---
> drivers/dma/idxd/cdev.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
> index 0423655f5a88..b00926abc69a 100644
> --- a/drivers/dma/idxd/cdev.c
> +++ b/drivers/dma/idxd/cdev.c
> @@ -165,7 +165,7 @@ static void idxd_cdev_dev_release(struct device *dev)
> struct idxd_wq *wq = idxd_cdev->wq;
>
> cdev_ctx = &ictx[wq->idxd->data->type];
> - ida_simple_remove(&cdev_ctx->minor_ida, idxd_cdev->minor);
> + ida_free(&cdev_ctx->minor_ida, idxd_cdev->minor);
> kfree(idxd_cdev);
> }
>
> @@ -463,7 +463,7 @@ int idxd_wq_add_cdev(struct idxd_wq *wq)
> cdev = &idxd_cdev->cdev;
> dev = cdev_dev(idxd_cdev);
> cdev_ctx = &ictx[wq->idxd->data->type];
> - minor = ida_simple_get(&cdev_ctx->minor_ida, 0, MINORMASK, GFP_KERNEL);
> + minor = ida_alloc_max(&cdev_ctx->minor_ida, MINORMASK, GFP_KERNEL);
> if (minor < 0) {
> kfree(idxd_cdev);
> return minor;

Thanks.

-Fenghua