[v2] bus/vdev: revert fix devargs after multi-process bus scan
Checks
Commit Message
This patch reverts commit f5b2eff0847d ("bus/vdev: fix devargs after
multi-process bus scan")
With current code, we do not add devargs to devargs list when we add a
vdev in secondary process (because `init` flag is set to `false`).
Because of this, when we do vdev_uninit, we call rte_devargs_remove on
the &devargs pointer (the one we didn't add to devargs list but did save
inside rte_vdev_device struct), but in secondary process, because
devargs were not added to the list in the first place, devargs_remove
does not find the associated devargs in its list and therefore does not
free associated resources. As a result, we get a memory leak, because we
free the rte_vdev_device but not its associated devargs.
Revert this patch to avoid leaking devargs on vdev uninit.
Fixes: f5b2eff0847d ("bus/vdev: fix devargs after multi-process bus scan")
Cc: stable@dpdk.org
Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
v2: Modify commit log.
---
drivers/bus/vdev/vdev.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
Comments
On Tue, May 13, 2025 at 11:52 AM Mingjin Ye <mingjinx.ye@intel.com> wrote:
>
> This patch reverts commit f5b2eff0847d ("bus/vdev: fix devargs after
> multi-process bus scan")
>
> With current code, we do not add devargs to devargs list when we add a
> vdev in secondary process (because `init` flag is set to `false`).
>
> Because of this, when we do vdev_uninit, we call rte_devargs_remove on
> the &devargs pointer (the one we didn't add to devargs list but did save
> inside rte_vdev_device struct), but in secondary process, because
> devargs were not added to the list in the first place, devargs_remove
> does not find the associated devargs in its list and therefore does not
> free associated resources. As a result, we get a memory leak, because we
> free the rte_vdev_device but not its associated devargs.
>
> Revert this patch to avoid leaking devargs on vdev uninit.
>
> Fixes: f5b2eff0847d ("bus/vdev: fix devargs after multi-process bus scan")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
I see back and forth on this topic but this still seems strange we
would have to revert such an old commit.
Please add a unit test showing the bug.
@@ -269,9 +269,7 @@ alloc_devargs(const char *name, const char *args)
}
static int
-insert_vdev(const char *name, const char *args,
- struct rte_vdev_device **p_dev,
- bool init)
+insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
{
struct rte_vdev_device *dev;
struct rte_devargs *devargs;
@@ -304,8 +302,7 @@ insert_vdev(const char *name, const char *args,
goto fail;
}
- if (init)
- rte_devargs_insert(&devargs);
+ rte_devargs_insert(&devargs);
dev->device.devargs = devargs;
dev->device.name = devargs->name;
TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
@@ -329,7 +326,7 @@ rte_vdev_init(const char *name, const char *args)
int ret;
rte_spinlock_recursive_lock(&vdev_device_list_lock);
- ret = insert_vdev(name, args, &dev, true);
+ ret = insert_vdev(name, args, &dev);
if (ret == 0) {
ret = vdev_probe_all_drivers(dev);
if (ret) {
@@ -456,7 +453,7 @@ vdev_action(const struct rte_mp_msg *mp_msg, const void *peer)
break;
case VDEV_SCAN_ONE:
VDEV_LOG(INFO, "receive vdev, %s", in->name);
- ret = insert_vdev(in->name, NULL, NULL, false);
+ ret = insert_vdev(in->name, NULL, NULL);
if (ret == -EEXIST)
VDEV_LOG(DEBUG, "device already exist, %s", in->name);
else if (ret < 0)