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.
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