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 [off-list ref]
wrote:

quoted

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

quoted

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)

quoted

  * don't appear on the trace path, they should be handled along with

the

quoted

  * 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)

quoted

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

quoted

+                               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)

quoted

        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,

quoted

        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,

quoted

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.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel