|SUCCESS| [v2, 2/2] dma/dpaa2: fix dpaa2_qdma_remove always returning success
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Sun May 17 15:56:29 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164051
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-17
commit b5721f271cbf, 8caf8427f85a
Author: Md Shofiqul Islam <shofiqtest at gmail.com>
Subject: [PATCH v2 1/2] bus/fslmc: fix ignored return value in fslmc_bus_unplug
This series fixes error handling in the fslmc bus unplug path and the
dpaa2 QDMA driver remove callback. Both patches address ignored return
values that prevented proper error propagation during device removal.
> diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
> index cf881b3eec..57bb61dc35 100644
> --- a/drivers/bus/fslmc/fslmc_bus.c
> +++ b/drivers/bus/fslmc/fslmc_bus.c
> @@ -620,7 +620,9 @@ fslmc_bus_unplug(struct rte_device *rte_dev)
> struct rte_dpaa2_driver *drv = dev->driver;
>
> if (drv && drv->remove) {
> - drv->remove(dev);
> + int ret = drv->remove(dev);
> + if (ret != 0)
> + return ret;
> dev->driver = NULL;
> dev->device.driver = NULL;
> DPAA2_BUS_INFO("%s Un-Plugged", dev->device.name);
Does this code leak the log message on the success path? After the new
early return on error, the DPAA2_BUS_INFO() call still happens only when
remove succeeds. However, should there be any cleanup or error logging
when the remove callback fails?
Looking at fslmc_bus_unplug(), if drv->remove() returns non-zero, does
the function leave the device in a partially unplugged state? The driver
pointers remain set, but should there be an error log indicating unplug
failure?
> diff --git a/drivers/dma/dpaa2/dpaa2_qdma.c b/drivers/dma/dpaa2/dpaa2_qdma.c
> index beca464c72..f7d94bb799 100644
> --- a/drivers/dma/dpaa2/dpaa2_qdma.c
> +++ b/drivers/dma/dpaa2/dpaa2_qdma.c
> @@ -1739,7 +1739,7 @@ dpaa2_qdma_remove(struct rte_dpaa2_device *dpaa2_dev)
> if (ret)
> DPAA2_QDMA_ERR("Device cleanup failed");
>
> - return 0;
> + return ret;
> }
In dpaa2_qdma_remove(), after rte_dma_pmd_release() fails and returns
non-zero, does the function leave any resources in an inconsistent
state? Should there be any attempt to proceed with partial cleanup even
when rte_dma_pmd_release() fails, or is the early failure return the
correct approach?
More information about the test-report
mailing list