Re: [PATCH v7 06/25] coresight: add try_get_module() in coresight_grab_device()
From: Suzuki K Poulose <suzuki.poulose@arm.com>
Date: 2020-08-05 10:52:15
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
quoted hunk ↗ jump to hunk
When coresight device is in an active session, driver module of that device should not be removed. Use try_get_module() in coresight_grab_device() to prevent module to be unloaded. Use get_device()/put_device() to protect device data in the middle of active session. Signed-off-by: Tingwei Zhang <redacted> Tested-by: Mike Leach <redacted> --- drivers/hwtracing/coresight/coresight.c | 39 +++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-)diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index b7151c5f81b1..1626bc885dfe 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c@@ -640,7 +640,7 @@ struct coresight_device *coresight_get_sink_by_id(u32 id) * don't appear on the trace path, they should be handled along with the * the master device. */ -static void coresight_grab_device(struct coresight_device *csdev) +static int coresight_grab_device(struct coresight_device *csdev) { int i;
Please could you add a pair of helper functions to hold/drop reference
to a device/driver ? i.e,
static inline bool coresight_get_ref(struct coresight_device *csdev)
{
struct device *dev = csdev->dev.parent;
/* Make sure the driver cant be removed */
if (!try_module_get(dev->driver->owner))
return false;
/* Make sure the device can't go away */
get_device(dev);
pm_runtime_get_sync(dev);
return true;
}
static inline void coresight_put_ref(struct coresight_device *csdev)
{
struct device *dev = csdev->dev.parent;
pm_runtime_put(dev);
put_device(dev);
module_put(dev->driver->owner);
}
quoted hunk ↗ jump to hunk
@@ -648,10 +648,30 @@ static void coresight_grab_device(struct coresight_device *csdev) struct coresight_device *child; child = csdev->pdata->conns[i].child_dev; - if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) + if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) { + if (!try_module_get(child->dev.parent->driver->owner)) + goto err; + get_device(child->dev.parent); pm_runtime_get_sync(child->dev.parent); + } }
This could then be : if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) if (!coresight_get_ref(child)) goto err;
+ if (!try_module_get(csdev->dev.parent->driver->owner)) + goto err; + get_device(csdev->dev.parent); pm_runtime_get_sync(csdev->dev.parent);
if (coresight_get_ref(csdev)) return 0;
+ return 0;
+err:
+ for (i--; i >= 0; i--) {
+ struct coresight_device *child;
+
+ child = csdev->pdata->conns[i].child_dev;
+ if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
+ pm_runtime_put(child->dev.parent);
+ put_device(child->dev.parent);
+ module_put(child->dev.parent->driver->owner);
+ }And this could be : if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) coresight_put_ref(child);
quoted hunk ↗ jump to hunk
+ } + return -ENODEV; } /*@@ -663,12 +683,17 @@ static void coresight_drop_device(struct coresight_device *csdev) int i; pm_runtime_put(csdev->dev.parent); + put_device(csdev->dev.parent); + module_put(csdev->dev.parent->driver->owner);
coresight_put_ref(csdev);
for (i = 0; i < csdev->pdata->nr_outport; i++) {
struct coresight_device *child;
child = csdev->pdata->conns[i].child_dev;
- if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
+ if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) {
pm_runtime_put(child->dev.parent);
+ put_device(child->dev.parent);
+ module_put(child->dev.parent->driver->owner);
+ }coresight_put_ref(child);
quoted hunk ↗ jump to hunk
} }@@ -687,7 +712,7 @@ static int _coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink, struct list_head *path) { - int i; + int i, ret; bool found = false; struct coresight_node *node;@@ -721,7 +746,11 @@ static int _coresight_build_path(struct coresight_device *csdev, if (!node) return -ENOMEM; - coresight_grab_device(csdev); + ret = coresight_grab_device(csdev); + if (ret) { + kfree(node); + return ret; + } node->csdev = csdev; list_add(&node->link, path);
Cheers Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel