Skip to contentSkip to conversation

bus/fslmc, dma/dpaa2: fix remove callback error propagation

PATCH Superseded Md Shofiqul Islam 2 participants 4 messages May 16 – May 18 dmaengine

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Md Shofiqul Islam

author
#0 lore raw

cover bus/fslmc, dma/dpaa2: fix remove callback error propagation

Two dpaa2 drivers silently discard errors from their remove callbacks, making it impossible for callers to detect cleanup failures:

- fslmc_bus_unplug() (bus/fslmc) calls drv->remove() and throws away the return value, reporting success regardless of outcome.

- dpaa2_qdma_remove() (dma/dpaa2) logs a cleanup error but always returns 0, hiding the failure from its caller.

This series fixes both by propagating the actual return values.

Bugzilla ID: 1914

Md Shofiqul Islam (2):
bus/fslmc: fix ignored return value in fslmc_bus_unplug dma/dpaa2: fix dpaa2_qdma_remove always returning success

 drivers/bus/fslmc/fslmc_bus.c  | 4 +++-
 drivers/dma/dpaa2/dpaa2_qdma.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)
Signature · 1 line
2.54.0.windows.1
author
#1 lore raw

PATCH 1/2 bus/fslmc: fix ignored return value in fslmc_bus_unplug

fslmc_bus_unplug() called drv->remove() and discarded the return value, unconditionally clearing driver references and reporting success even when the remove callback signalled failure. As a result, callers had no way to detect or react to removal errors.

Capture the return value and propagate it to the caller. Only clear the driver references and log successful unplug when the callback returns zero.

Fixes b5721f271cbf ("bus/fslmc: support DPNI hotplug")

Bugzilla ID: 1914
Cc: [email protected]
Cc: [email protected]

Signed-off-by Md Shofiqul Islam <[email protected]>
---
 drivers/bus/fslmc/fslmc_bus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/bus/fslmc/fslmc_bus.c +3 -1
index cf881b3eec..9cfd8b10ba 100644
@@ -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)
+			return ret;
 		dev->driver = NULL;
 		dev->device.driver = NULL;
 		DPAA2_BUS_INFO("%s Un-Plugged",  dev->device.name);
Signature · 1 line
2.54.0.windows.1
#2 lore raw

Re: [PATCH 1/2] bus/fslmc: fix ignored return value in fslmc_bus_unplug

Hi Shofiqul,

Quoted 25 lines · Md Shofiqul Islam

fslmc_bus_unplug() called drv->remove() and discarded the return value, unconditionally clearing driver references and reporting success even when the remove callback signalled failure. As a result, callers had no way to detect or react to removal errors.

Capture the return value and propagate it to the caller. Only clear the driver references and log successful unplug when the callback returns zero.

Fixes b5721f271cbf ("bus/fslmc: support DPNI hotplug")

Bugzilla ID: 1914
Cc: [email protected]
Cc: [email protected]

Signed-off-by Md Shofiqul Islam <[email protected]>
---
 drivers/bus/fslmc/fslmc_bus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/bus/fslmc/fslmc_bus.c +0 -0
index cf881b3eec..9cfd8b10ba 100644

drivers/bus/fslmc/fslmc_bus.c fslmc_bus_unplug(struct rte_device *rte_dev) · line 624

Quoted 6 lines
drivers/bus/fslmc/fslmc_bus.c +2 -1
@@ -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)

We log when the function successfully unplugs the driver, but there is no log for error. Adding one could help here.

drivers/bus/fslmc/fslmc_bus.c fslmc_bus_unplug(struct rte_device *rte_dev) · line 625

Quoted 1 line
drivers/bus/fslmc/fslmc_bus.c +1 -0
+                       return ret;

Could you please declare ret at the top of the function to match the existing style in this file (fslmc_bus_plug() just above, plus scan_one_fslmc_device, rte_fslmc_parse, rte_fslmc_scan, rte_fslmc_close all declare int ret in the local-variable block) ?

drivers/bus/fslmc/fslmc_bus.c fslmc_bus_unplug(struct rte_device *rte_dev) · line 628

Quoted 3 lines
drivers/bus/fslmc/fslmc_bus.c +0 -0
                dev->driver = NULL;
                dev->device.driver = NULL;
                DPAA2_BUS_INFO("%s Un-Plugged",  dev->device.name);

Regards,

Maxime

author
#3 lore raw

PATCH 2/2 dma/dpaa2: fix dpaa2_qdma_remove always returning success

In reply to Md Shofiqul Islam

dpaa2_qdma_remove() checked the return value of rte_dma_pmd_release() and logged an error on failure, but then unconditionally returned 0, hiding the failure from fslmc_bus_unplug() and any higher-level caller. A device-cleanup failure was silently swallowed.

Return the actual error code so that callers can detect and handle removal failures correctly.

Fixes 8caf8427f85a ("dma/dpaa2: introduce driver skeleton")

Bugzilla ID: 1914
Cc: [email protected]
Cc: [email protected]

Signed-off-by Md Shofiqul Islam <[email protected]>
---
 drivers/dma/dpaa2/dpaa2_qdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/dma/dpaa2/dpaa2_qdma.c +1 -1
index beca464c72..f7d94bb799 100644
@@ -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;
 }
 
 static struct rte_dpaa2_driver rte_dpaa2_qdma_pmd;
Signature · 1 line
2.54.0.windows.1