Hi Tingwei,
On Sat, 25 Jul 2020 at 11:05, Tingwei Zhang tingweiz@codeaurora.org wrote:
Hi Mike,
Thu, Jul 23, 2020 at 06:55:47PM +0800, Mike Leach wrote:
Hi Tingwei,
On Thu, 23 Jul 2020 at 05:28, Tingwei Zhang tingwei@codeaurora.org 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.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org
drivers/hwtracing/coresight/coresight.c | 27 +++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c
b/drivers/hwtracing/coresight/coresight.c
index b7151c5f81b1..17bc76ea86ae 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;
@@ -648,10 +648,25 @@ 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; pm_runtime_get_sync(child->dev.parent);} }if (!try_module_get(csdev->dev.parent->driver->owner))goto err; pm_runtime_get_sync(csdev->dev.parent);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);}return -ENODEV;}
/* @@ -663,12 +678,15 @@ static void coresight_drop_device(struct
coresight_device *csdev)
int i; pm_runtime_put(csdev->dev.parent);
module_put(csdev->dev.parent->driver->owner); 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);module_put(child->dev.parent->driver->owner);} }}
@@ -721,7 +739,8 @@ static int _coresight_build_path(struct
coresight_device *csdev,
if (!node) return -ENOMEM;
coresight_grab_device(csdev);
if (coresight_grab_device(csdev))return -ENODEV; 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
The CTI devices are associated with other coresight components using csdev->ect_dev; These are not handled in the main linear trace path as these have a star topology interlinking all components. However, when a component has an associated ect_dev then is enabled at the same time as the other component is enabled.
So a module_get/put will be needed for this association to prevent the CTI being removed while a trace session is active. I suggest in coresight.c : coresight_control_assoc_ectdev()
In module unload process, devices of that module will be removed. In this case, cti_remove() is called on all cti device when unloading cti module. All cti connections is cleaned at that time. csdev->ect_dev is set to NULL. coresight_control_assoc_ectdev() will return since since ect_csdev is NULL.
I think we are safe without holding module reference since cti driver has done a pretty good job on clean up.
The issue here is not about clean up. We need to keep all the programmed coresight modules loaded and operational while a coresight trace session is in progress. The CTI can be used to control the generation of trace, trigger trace related events etc. If the module is removed while a session is in progress then this programming is lost and the trace data collected may not be what is expected.
Regards
Mike
Let me know if you still think we need hold the module reference.
Thanks, Tingwei
Regards
Mike
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel