On Thu, Aug 13, 2020 at 03:36:12AM +0800, Mathieu Poirier wrote:
On Fri, Aug 07, 2020 at 07:11:35PM +0800, Tingwei Zhang wrote:
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 tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org Suggested-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight.c | 61 ++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c
b/drivers/hwtracing/coresight/coresight.c
index b7151c5f81b1..0b0e31577b9b 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -634,13 +634,45 @@ struct coresight_device
*coresight_get_sink_by_id(u32 id)
return dev ? to_coresight_device(dev) : NULL; } +/**
- coresight_get_ref- Helper function to increase reference count to
module
- and device.
- Return true in successful case and power up the device.
- Return false when failed to get reference of module.
- */
+static inline bool coresight_get_ref(struct coresight_device *csdev) +{
- struct device *dev = csdev->dev.parent;
- /* Make sure the driver can't 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;
+}
+/**
- coresight_put_ref- Helper function to decrease reference count to
module
- and device. Power off the device.
- */
+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);
+}
/*
- coresight_grab_device - Power up this device and any of the helper
- devices connected to it for trace operation. Since the helper
devices
- 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; @@ -649,9 +681,20 @@ static void coresight_grab_device(struct
coresight_device *csdev)
child = csdev->pdata->conns[i].child_dev; if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
pm_runtime_get_sync(child->dev.parent);
if (!coresight_get_ref(child))
goto err;
- }
- if (coresight_get_ref(csdev))
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)
}coresight_put_ref(child);
- pm_runtime_get_sync(csdev->dev.parent);
- return -ENODEV;
} /* @@ -662,13 +705,13 @@ static void coresight_drop_device(struct
coresight_device *csdev)
{ int i;
- pm_runtime_put(csdev->dev.parent);
- 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)
pm_runtime_put(child->dev.parent);
}coresight_put_ref(child);
} @@ -687,7 +730,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 +764,11 @@ static int _coresight_build_path(struct
coresight_device *csdev,
if (!node) return -ENOMEM;
- coresight_grab_device(csdev);
- ret = coresight_grab_device(csdev);
I suggest trying to grab the device before allocating memory for the node. If grabing the device fails memory doesn't have to be allocated and released needlessly.
Agree. I'll fix in next revision.
Thanks, Tingwei
I will likely come back to this patch later after reviewing the rest the this set.
Thanks, Mathieu
- if (ret) {
kfree(node);
return ret;
- } node->csdev = csdev; list_add(&node->link, path);
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project