[PATCH v7 06/25] coresight: add try_get_module() in coresight_grab_device()
Tingwei Zhang
tingweiz at codeaurora.org
Wed Aug 5 22:43:39 EDT 2020
On Wed, Aug 05, 2020 at 06:55:40PM +0800, Suzuki K Poulose wrote:
> On 08/05/2020 03:54 AM, 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 at codeaurora.org>
> >Tested-by: Mike Leach <mike.leach at linaro.org>
> >---
> > 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;
> }
>
Sure. I'll add it to next revision.
Thanks,
Tingwei
> 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);
> }
>
>
> >@@ -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);
>
> >+ }
> >+ 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);
>
> > }
> > }
> >@@ -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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
More information about the linux-arm-kernel
mailing list