2024-01-11 21:10:55

by Christophe JAILLET

[permalink] [raw]
Subject: [PATCH] thunderbolt: 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().

Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_range()/ida_alloc_max() is inclusive. So a -1 has been added
when needed.

Signed-off-by: Christophe JAILLET <[email protected]>
---
drivers/thunderbolt/domain.c | 6 +++---
drivers/thunderbolt/nhi.c | 6 +++---
drivers/thunderbolt/nvm.c | 4 ++--
drivers/thunderbolt/switch.c | 6 +++---
drivers/thunderbolt/xdomain.c | 4 ++--
5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 9fb1a64f3300..c067f9442694 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -321,7 +321,7 @@ static void tb_domain_release(struct device *dev)

tb_ctl_free(tb->ctl);
destroy_workqueue(tb->wq);
- ida_simple_remove(&tb_domain_ida, tb->index);
+ ida_free(&tb_domain_ida, tb->index);
mutex_destroy(&tb->lock);
kfree(tb);
}
@@ -389,7 +389,7 @@ struct tb *tb_domain_alloc(struct tb_nhi *nhi, int timeout_msec, size_t privsize
tb->nhi = nhi;
mutex_init(&tb->lock);

- tb->index = ida_simple_get(&tb_domain_ida, 0, 0, GFP_KERNEL);
+ tb->index = ida_alloc(&tb_domain_ida, GFP_KERNEL);
if (tb->index < 0)
goto err_free;

@@ -413,7 +413,7 @@ struct tb *tb_domain_alloc(struct tb_nhi *nhi, int timeout_msec, size_t privsize
err_destroy_wq:
destroy_workqueue(tb->wq);
err_remove_ida:
- ida_simple_remove(&tb_domain_ida, tb->index);
+ ida_free(&tb_domain_ida, tb->index);
err_free:
kfree(tb);

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index fb4f46e51753..e77a95aea066 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -465,7 +465,7 @@ static int ring_request_msix(struct tb_ring *ring, bool no_suspend)
if (!nhi->pdev->msix_enabled)
return 0;

- ret = ida_simple_get(&nhi->msix_ida, 0, MSIX_MAX_VECS, GFP_KERNEL);
+ ret = ida_alloc_max(&nhi->msix_ida, MSIX_MAX_VECS - 1, GFP_KERNEL);
if (ret < 0)
return ret;

@@ -485,7 +485,7 @@ static int ring_request_msix(struct tb_ring *ring, bool no_suspend)
return 0;

err_ida_remove:
- ida_simple_remove(&nhi->msix_ida, ring->vector);
+ ida_free(&nhi->msix_ida, ring->vector);

return ret;
}
@@ -496,7 +496,7 @@ static void ring_release_msix(struct tb_ring *ring)
return;

free_irq(ring->irq, ring);
- ida_simple_remove(&ring->nhi->msix_ida, ring->vector);
+ ida_free(&ring->nhi->msix_ida, ring->vector);
ring->vector = 0;
ring->irq = 0;
}
diff --git a/drivers/thunderbolt/nvm.c b/drivers/thunderbolt/nvm.c
index 69fb3b0fa34f..8901db2de327 100644
--- a/drivers/thunderbolt/nvm.c
+++ b/drivers/thunderbolt/nvm.c
@@ -330,7 +330,7 @@ struct tb_nvm *tb_nvm_alloc(struct device *dev)
if (!nvm)
return ERR_PTR(-ENOMEM);

- ret = ida_simple_get(&nvm_ida, 0, 0, GFP_KERNEL);
+ ret = ida_alloc(&nvm_ida, GFP_KERNEL);
if (ret < 0) {
kfree(nvm);
return ERR_PTR(ret);
@@ -528,7 +528,7 @@ void tb_nvm_free(struct tb_nvm *nvm)
nvmem_unregister(nvm->non_active);
nvmem_unregister(nvm->active);
vfree(nvm->buf);
- ida_simple_remove(&nvm_ida, nvm->id);
+ ida_free(&nvm_ida, nvm->id);
}
kfree(nvm);
}
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 900114ba4371..eb2d33091012 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -771,7 +771,7 @@ static int tb_port_alloc_hopid(struct tb_port *port, bool in, int min_hopid,
if (max_hopid < 0 || max_hopid > port_max_hopid)
max_hopid = port_max_hopid;

- return ida_simple_get(ida, min_hopid, max_hopid + 1, GFP_KERNEL);
+ return ida_alloc_range(ida, min_hopid, max_hopid, GFP_KERNEL);
}

/**
@@ -809,7 +809,7 @@ int tb_port_alloc_out_hopid(struct tb_port *port, int min_hopid, int max_hopid)
*/
void tb_port_release_in_hopid(struct tb_port *port, int hopid)
{
- ida_simple_remove(&port->in_hopids, hopid);
+ ida_free(&port->in_hopids, hopid);
}

/**
@@ -819,7 +819,7 @@ void tb_port_release_in_hopid(struct tb_port *port, int hopid)
*/
void tb_port_release_out_hopid(struct tb_port *port, int hopid)
{
- ida_simple_remove(&port->out_hopids, hopid);
+ ida_free(&port->out_hopids, hopid);
}

static inline bool tb_switch_is_reachable(const struct tb_switch *parent,
diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 9495742913d5..8a5544243d56 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -997,7 +997,7 @@ static void tb_service_release(struct device *dev)
struct tb_xdomain *xd = tb_service_parent(svc);

tb_service_debugfs_remove(svc);
- ida_simple_remove(&xd->service_ids, svc->id);
+ ida_free(&xd->service_ids, svc->id);
kfree(svc->key);
kfree(svc);
}
@@ -1099,7 +1099,7 @@ static void enumerate_services(struct tb_xdomain *xd)
break;
}

- id = ida_simple_get(&xd->service_ids, 0, 0, GFP_KERNEL);
+ id = ida_alloc(&xd->service_ids, GFP_KERNEL);
if (id < 0) {
kfree(svc->key);
kfree(svc);
--
2.34.1



2024-01-22 18:27:09

by Christophe JAILLET

[permalink] [raw]
Subject: Re: [PATCH] thunderbolt: Remove usage of the deprecated ida_simple_xx() API

Le 22/01/2024 à 12:29, Mika Westerberg a écrit :
> On Thu, Jan 11, 2024 at 10:10:21PM +0100, Christophe JAILLET wrote:
>> ida_alloc() and ida_free() should be preferred to the deprecated
>> ida_simple_get() and ida_simple_remove().
>>
>> Note that the upper limit of ida_simple_get() is exclusive, but the one of
>> ida_alloc_range()/ida_alloc_max() is inclusive. So a -1 has been added
>> when needed.
>
> Looks tood to me but wanted to check if you tested this on a real
> hardware or you just build tested?
>
>

Hi,

It was compile tested only.

Transformation has been done with the help of the cocci script below.

CJ


===

@@
expression i, gfp;
@@
- ida_simple_get(i, 0, 0, gfp)
+ ida_alloc(i, gfp)


@@
expression e1, e2, gfp;
@@
- ida_simple_get(e1, e2, 0, gfp)
+ ida_alloc_min(e1, e2, gfp)


@@
expression e1, e2, gfp;
@@
- ida_simple_get(e1, 0, e2, gfp)
+ ida_alloc_max(e1, e2 - 1, gfp)


@@
expression e1, e2, gfp;
@@
- ida_simple_get(e1, e2, e2+1, gfp)
+ ida_alloc_range(e1, e2, e2, gfp)


@@
expression e1, e2, e3, gfp;
@@
- ida_simple_get(e1, e2, e3, gfp)
+ ida_alloc_range(e1, e2, e3 - 1, gfp)


@@
expression e1, e2;
@@
- ida_simple_remove(e1, e2)
+ ida_free(e1, e2)



2024-01-22 20:38:35

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH] thunderbolt: Remove usage of the deprecated ida_simple_xx() API

On Thu, Jan 11, 2024 at 10:10:21PM +0100, Christophe JAILLET wrote:
> ida_alloc() and ida_free() should be preferred to the deprecated
> ida_simple_get() and ida_simple_remove().
>
> Note that the upper limit of ida_simple_get() is exclusive, but the one of
> ida_alloc_range()/ida_alloc_max() is inclusive. So a -1 has been added
> when needed.

Looks tood to me but wanted to check if you tested this on a real
hardware or you just build tested?

2024-01-23 15:38:02

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH] thunderbolt: Remove usage of the deprecated ida_simple_xx() API

On Mon, Jan 22, 2024 at 06:57:42PM +0100, Christophe JAILLET wrote:
> Le 22/01/2024 à 12:29, Mika Westerberg a écrit :
> > On Thu, Jan 11, 2024 at 10:10:21PM +0100, Christophe JAILLET wrote:
> > > ida_alloc() and ida_free() should be preferred to the deprecated
> > > ida_simple_get() and ida_simple_remove().
> > >
> > > Note that the upper limit of ida_simple_get() is exclusive, but the one of
> > > ida_alloc_range()/ida_alloc_max() is inclusive. So a -1 has been added
> > > when needed.
> >
> > Looks tood to me but wanted to check if you tested this on a real
> > hardware or you just build tested?
> >
> >
>
> Hi,
>
> It was compile tested only.
>
> Transformation has been done with the help of the cocci script below.

Okay that's what I thought too. I ran some testing on a real hardware
and did not see any issues.

Applied to thunderbolt.git/next, thanks!