The TRBE driver support is build as a module, we found some driver issues
based on the patchset [1] and set CONFIG_CORESIGHT_TRBE=m.
1. TRBE driver potential sleep in atomic context when unregister device
2. Multiple free the platform data resource when rmmod coresight TRBE
driver
[1] "coresight: trbe: Enable ACPI based devices"
https://lore.kernel.org/all/[email protected]/
Junhao He (2):
coresight: trbe: Fix TRBE potential sleep in atomic context
coresight: core: Fix multiple free TRBE platform data resource
drivers/hwtracing/coresight/coresight-core.c | 7 ++--
drivers/hwtracing/coresight/coresight-trbe.c | 35 +++++++++++---------
2 files changed, 24 insertions(+), 18 deletions(-)
--
2.33.0
From: Junhao He <[email protected]>
smp_call_function_single() will allocate an IPI interrupt vector to
the target processor and send a function call request to the interrupt
vector. After the target processor receives the IPI interrupt, it will
execute arm_trbe_remove_coresight_cpu() call request in the interrupt
handler.
According to the device_unregister() stack information, if other process
is useing the device, the down_write() may sleep, and trigger deadlocks
or unexpected errors.
arm_trbe_remove_coresight_cpu
coresight_unregister
device_unregister
device_del
kobject_del
__kobject_del
sysfs_remove_dir
kernfs_remove
down_write ---------> it may sleep
Add a helper arm_trbe_disable_cpu() to disable TRBE precpu irq and reset
per TRBE.
Simply call arm_trbe_remove_coresight_cpu() directly without useing the
smp_call_function_single(), which is the same as registering the TRBE
coresight device.
Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
Signed-off-by: Junhao He <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[ Remove duplicate cpumask checks during removal ]
Signed-off-by: Suzuki K Poulose <[email protected]>
---
drivers/hwtracing/coresight/coresight-trbe.c | 33 +++++++++++---------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 7720619909d6..025f70adee47 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -1225,6 +1225,17 @@ static void arm_trbe_enable_cpu(void *info)
enable_percpu_irq(drvdata->irq, IRQ_TYPE_NONE);
}
+static void arm_trbe_disable_cpu(void *info)
+{
+ struct trbe_drvdata *drvdata = info;
+ struct trbe_cpudata *cpudata = this_cpu_ptr(drvdata->cpudata);
+
+ disable_percpu_irq(drvdata->irq);
+ trbe_reset_local(cpudata);
+ cpudata->drvdata = NULL;
+}
+
+
static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cpu)
{
struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
@@ -1326,18 +1337,12 @@ static void arm_trbe_probe_cpu(void *info)
cpumask_clear_cpu(cpu, &drvdata->supported_cpus);
}
-static void arm_trbe_remove_coresight_cpu(void *info)
+static void arm_trbe_remove_coresight_cpu(struct trbe_drvdata *drvdata, int cpu)
{
- int cpu = smp_processor_id();
- struct trbe_drvdata *drvdata = info;
- struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
struct coresight_device *trbe_csdev = coresight_get_percpu_sink(cpu);
- disable_percpu_irq(drvdata->irq);
- trbe_reset_local(cpudata);
if (trbe_csdev) {
coresight_unregister(trbe_csdev);
- cpudata->drvdata = NULL;
coresight_set_percpu_sink(cpu, NULL);
}
}
@@ -1366,8 +1371,10 @@ static int arm_trbe_remove_coresight(struct trbe_drvdata *drvdata)
{
int cpu;
- for_each_cpu(cpu, &drvdata->supported_cpus)
- smp_call_function_single(cpu, arm_trbe_remove_coresight_cpu, drvdata, 1);
+ for_each_cpu(cpu, &drvdata->supported_cpus) {
+ smp_call_function_single(cpu, arm_trbe_disable_cpu, drvdata, 1);
+ arm_trbe_remove_coresight_cpu(drvdata, cpu);
+ }
free_percpu(drvdata->cpudata);
return 0;
}
@@ -1406,12 +1413,8 @@ static int arm_trbe_cpu_teardown(unsigned int cpu, struct hlist_node *node)
{
struct trbe_drvdata *drvdata = hlist_entry_safe(node, struct trbe_drvdata, hotplug_node);
- if (cpumask_test_cpu(cpu, &drvdata->supported_cpus)) {
- struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
-
- disable_percpu_irq(drvdata->irq);
- trbe_reset_local(cpudata);
- }
+ if (cpumask_test_cpu(cpu, &drvdata->supported_cpus))
+ arm_trbe_disable_cpu(drvdata);
return 0;
}
--
2.34.1
Coresight TRBE driver shares a single platform data (which is empty btw).
However, with the commit 4e8fe7e5c3a5
("coresight: Store pointers to connections rather than an array of them")
the coresight core would free up the pdata, resulting in multiple attempts
to free the same pdata for TRBE instances. Fix this by allocating a pdata per
coresight_device.
Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
Link: https://lore.kernel.org/r/[email protected]
Reported-by: Junhao He <[email protected]>
Cc: Anshuman Khandual <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
drivers/hwtracing/coresight/coresight-trbe.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 025f70adee47..d3d34a833f01 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -1255,10 +1255,13 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp
if (!desc.name)
goto cpu_clear;
+ desc.pdata = coresight_get_platform_data(dev);
+ if (IS_ERR(desc.pdata))
+ goto cpu_clear;
+
desc.type = CORESIGHT_DEV_TYPE_SINK;
desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM;
desc.ops = &arm_trbe_cs_ops;
- desc.pdata = dev_get_platdata(dev);
desc.groups = arm_trbe_groups;
desc.dev = dev;
trbe_csdev = coresight_register(&desc);
@@ -1482,7 +1485,6 @@ static void arm_trbe_remove_irq(struct trbe_drvdata *drvdata)
static int arm_trbe_device_probe(struct platform_device *pdev)
{
- struct coresight_platform_data *pdata;
struct trbe_drvdata *drvdata;
struct device *dev = &pdev->dev;
int ret;
@@ -1497,12 +1499,7 @@ static int arm_trbe_device_probe(struct platform_device *pdev)
if (!drvdata)
return -ENOMEM;
- pdata = coresight_get_platform_data(dev);
- if (IS_ERR(pdata))
- return PTR_ERR(pdata);
-
dev_set_drvdata(dev, drvdata);
- dev->platform_data = pdata;
drvdata->pdev = pdev;
ret = arm_trbe_probe_irq(pdev, drvdata);
if (ret)
--
2.34.1
On 17/08/2023 07:37, Anshuman Khandual wrote:
> Hi Suzuki,
>
> Seems like this patch is going to conflict with the below proposed change
>
> https://lore.kernel.org/all/[email protected]/
>
> Please let me know how should we resolve this conflict.
We could merge them both, with the fixes: one first, just to acknowledge
that there was a problem. But I suppose your one will have to be rebased
on top.
>
> On 8/16/23 19:40, Suzuki K Poulose wrote:
>> Coresight TRBE driver shares a single platform data (which is empty btw).
>> However, with the commit 4e8fe7e5c3a5
>> ("coresight: Store pointers to connections rather than an array of them")
>> the coresight core would free up the pdata, resulting in multiple attempts
>> to free the same pdata for TRBE instances. Fix this by allocating a pdata per
>> coresight_device.
>>
>> Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
>
> The above mentioned commit i.e 4e8fe7e5c3a5 seems to be a more recent one which
> has triggered this problem. But would the problem be still there without that ?
> Else 'Fixes:' tag would need changing.
>
Yes I think the fixes tag should point to 4e8fe7e5c3a5.
>> Link: https://lore.kernel.org/r/[email protected]
>> Reported-by: Junhao He <[email protected]>
>> Cc: Anshuman Khandual <[email protected]>
>> Signed-off-by: Suzuki K Poulose <[email protected]>
>> ---
>> drivers/hwtracing/coresight/coresight-trbe.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 025f70adee47..d3d34a833f01 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -1255,10 +1255,13 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp
>> if (!desc.name)
>> goto cpu_clear;
>>
>> + desc.pdata = coresight_get_platform_data(dev);
>> + if (IS_ERR(desc.pdata))
>> + goto cpu_clear;
>> +
>> desc.type = CORESIGHT_DEV_TYPE_SINK;
>> desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM;
>> desc.ops = &arm_trbe_cs_ops;
>> - desc.pdata = dev_get_platdata(dev);
>> desc.groups = arm_trbe_groups;
>> desc.dev = dev;
>> trbe_csdev = coresight_register(&desc);
>> @@ -1482,7 +1485,6 @@ static void arm_trbe_remove_irq(struct trbe_drvdata *drvdata)
>>
>> static int arm_trbe_device_probe(struct platform_device *pdev)
>> {
>> - struct coresight_platform_data *pdata;
>> struct trbe_drvdata *drvdata;
>> struct device *dev = &pdev->dev;
>> int ret;
>> @@ -1497,12 +1499,7 @@ static int arm_trbe_device_probe(struct platform_device *pdev)
>> if (!drvdata)
>> return -ENOMEM;
>>
>> - pdata = coresight_get_platform_data(dev);
>> - if (IS_ERR(pdata))
>> - return PTR_ERR(pdata);
>> -
>> dev_set_drvdata(dev, drvdata);
>> - dev->platform_data = pdata;
>> drvdata->pdev = pdev;
>> ret = arm_trbe_probe_irq(pdev, drvdata);
>> if (ret)
Hello Junhao,
On 8/16/23 19:40, Suzuki K Poulose wrote:
> From: Junhao He <[email protected]>
>
> smp_call_function_single() will allocate an IPI interrupt vector to
> the target processor and send a function call request to the interrupt
> vector. After the target processor receives the IPI interrupt, it will
> execute arm_trbe_remove_coresight_cpu() call request in the interrupt
> handler.
>
> According to the device_unregister() stack information, if other process
> is useing the device, the down_write() may sleep, and trigger deadlocks
> or unexpected errors.
>
> arm_trbe_remove_coresight_cpu
> coresight_unregister
> device_unregister
> device_del
> kobject_del
> __kobject_del
> sysfs_remove_dir
> kernfs_remove
> down_write ---------> it may sleep
But how did you really detect this problem ? Does this show up as an warning when
you enable lockdep debug ? OR it really happened during a real workload execution
followed by TRBE module unload. Although the problem seems plausible (which needs
fixing), just wondering how did we trigger this.
>
> Add a helper arm_trbe_disable_cpu() to disable TRBE precpu irq and reset
> per TRBE.
> Simply call arm_trbe_remove_coresight_cpu() directly without useing the
> smp_call_function_single(), which is the same as registering the TRBE
> coresight device.
>
> Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
> Signed-off-by: Junhao He <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> [ Remove duplicate cpumask checks during removal ]
> Signed-off-by: Suzuki K Poulose <[email protected]>
> ---
> drivers/hwtracing/coresight/coresight-trbe.c | 33 +++++++++++---------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 7720619909d6..025f70adee47 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -1225,6 +1225,17 @@ static void arm_trbe_enable_cpu(void *info)
> enable_percpu_irq(drvdata->irq, IRQ_TYPE_NONE);
> }
>
> +static void arm_trbe_disable_cpu(void *info)
> +{
> + struct trbe_drvdata *drvdata = info;
> + struct trbe_cpudata *cpudata = this_cpu_ptr(drvdata->cpudata);
> +
> + disable_percpu_irq(drvdata->irq);
> + trbe_reset_local(cpudata);
> + cpudata->drvdata = NULL;
> +}
> +
> +
> static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cpu)
> {
> struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
> @@ -1326,18 +1337,12 @@ static void arm_trbe_probe_cpu(void *info)
> cpumask_clear_cpu(cpu, &drvdata->supported_cpus);
> }
>
> -static void arm_trbe_remove_coresight_cpu(void *info)
> +static void arm_trbe_remove_coresight_cpu(struct trbe_drvdata *drvdata, int cpu)
> {
> - int cpu = smp_processor_id();
> - struct trbe_drvdata *drvdata = info;
> - struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
> struct coresight_device *trbe_csdev = coresight_get_percpu_sink(cpu);
>
> - disable_percpu_irq(drvdata->irq);
> - trbe_reset_local(cpudata);
> if (trbe_csdev) {
> coresight_unregister(trbe_csdev);
> - cpudata->drvdata = NULL;
> coresight_set_percpu_sink(cpu, NULL);
> }
> }
> @@ -1366,8 +1371,10 @@ static int arm_trbe_remove_coresight(struct trbe_drvdata *drvdata)
> {
> int cpu;
>
> - for_each_cpu(cpu, &drvdata->supported_cpus)
> - smp_call_function_single(cpu, arm_trbe_remove_coresight_cpu, drvdata, 1);
> + for_each_cpu(cpu, &drvdata->supported_cpus) {
> + smp_call_function_single(cpu, arm_trbe_disable_cpu, drvdata, 1);
> + arm_trbe_remove_coresight_cpu(drvdata, cpu);
> + }
> free_percpu(drvdata->cpudata);
> return 0;
> }
> @@ -1406,12 +1413,8 @@ static int arm_trbe_cpu_teardown(unsigned int cpu, struct hlist_node *node)
> {
> struct trbe_drvdata *drvdata = hlist_entry_safe(node, struct trbe_drvdata, hotplug_node);
>
> - if (cpumask_test_cpu(cpu, &drvdata->supported_cpus)) {
> - struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
> -
> - disable_percpu_irq(drvdata->irq);
> - trbe_reset_local(cpudata);
> - }
> + if (cpumask_test_cpu(cpu, &drvdata->supported_cpus))
> + arm_trbe_disable_cpu(drvdata);
This code hunk seems unrelated to the context here other than just finding another use case
for arm_trbe_disable_cpu(). The problem is - arm_trbe_disable_cpu() resets cpudata->drvdata
which might not get re-initialized back in arm_trbe_cpu_startup(), as there will still be a
per cpu sink associated as confirmed with coresight_get_percpu_sink(). I guess it might be
better to drop this change and just keep everything limited to SMP IPI callback reworking in
arm_trbe_remove_coresight().
> return 0;
> }
>
On 8/14/23 15:08, Junhao He wrote:
> The TRBE driver support is build as a module, we found some driver issues
> based on the patchset [1] and set CONFIG_CORESIGHT_TRBE=m.
> 1. TRBE driver potential sleep in atomic context when unregister device
> 2. Multiple free the platform data resource when rmmod coresight TRBE
> driver
>
> [1] "coresight: trbe: Enable ACPI based devices"
> https://lore.kernel.org/all/[email protected]/
I am glad that ACPI based device enablement is already helping in getting more
test coverage for the TRBE driver itself. I have just posted a new version for
the above series.
https://lore.kernel.org/all/[email protected]/
On 17/08/2023 08:13, Anshuman Khandual wrote:
> Hello Junhao,
>
> On 8/16/23 19:40, Suzuki K Poulose wrote:
>> From: Junhao He <[email protected]>
>>
>> smp_call_function_single() will allocate an IPI interrupt vector to
>> the target processor and send a function call request to the interrupt
>> vector. After the target processor receives the IPI interrupt, it will
>> execute arm_trbe_remove_coresight_cpu() call request in the interrupt
>> handler.
>>
>> According to the device_unregister() stack information, if other process
>> is useing the device, the down_write() may sleep, and trigger deadlocks
>> or unexpected errors.
>>
>> arm_trbe_remove_coresight_cpu
>> coresight_unregister
>> device_unregister
>> device_del
>> kobject_del
>> __kobject_del
>> sysfs_remove_dir
>> kernfs_remove
>> down_write ---------> it may sleep
>
> But how did you really detect this problem ? Does this show up as an warning when
> you enable lockdep debug ? OR it really happened during a real workload execution
> followed by TRBE module unload. Although the problem seems plausible (which needs
> fixing), just wondering how did we trigger this.
I was able to trigger this with just :
modprobe coresight-trbe; modprobe -r coresight-trbe;
With all the bells and whistles enabled in the kernel.
Suzuki
Hi Suzuki,
Seems like this patch is going to conflict with the below proposed change
https://lore.kernel.org/all/[email protected]/
Please let me know how should we resolve this conflict.
On 8/16/23 19:40, Suzuki K Poulose wrote:
> Coresight TRBE driver shares a single platform data (which is empty btw).
> However, with the commit 4e8fe7e5c3a5
> ("coresight: Store pointers to connections rather than an array of them")
> the coresight core would free up the pdata, resulting in multiple attempts
> to free the same pdata for TRBE instances. Fix this by allocating a pdata per
> coresight_device.
>
> Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
The above mentioned commit i.e 4e8fe7e5c3a5 seems to be a more recent one which
has triggered this problem. But would the problem be still there without that ?
Else 'Fixes:' tag would need changing.
> Link: https://lore.kernel.org/r/[email protected]
> Reported-by: Junhao He <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Signed-off-by: Suzuki K Poulose <[email protected]>
> ---
> drivers/hwtracing/coresight/coresight-trbe.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 025f70adee47..d3d34a833f01 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -1255,10 +1255,13 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp
> if (!desc.name)
> goto cpu_clear;
>
> + desc.pdata = coresight_get_platform_data(dev);
> + if (IS_ERR(desc.pdata))
> + goto cpu_clear;
> +
> desc.type = CORESIGHT_DEV_TYPE_SINK;
> desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM;
> desc.ops = &arm_trbe_cs_ops;
> - desc.pdata = dev_get_platdata(dev);
> desc.groups = arm_trbe_groups;
> desc.dev = dev;
> trbe_csdev = coresight_register(&desc);
> @@ -1482,7 +1485,6 @@ static void arm_trbe_remove_irq(struct trbe_drvdata *drvdata)
>
> static int arm_trbe_device_probe(struct platform_device *pdev)
> {
> - struct coresight_platform_data *pdata;
> struct trbe_drvdata *drvdata;
> struct device *dev = &pdev->dev;
> int ret;
> @@ -1497,12 +1499,7 @@ static int arm_trbe_device_probe(struct platform_device *pdev)
> if (!drvdata)
> return -ENOMEM;
>
> - pdata = coresight_get_platform_data(dev);
> - if (IS_ERR(pdata))
> - return PTR_ERR(pdata);
> -
> dev_set_drvdata(dev, drvdata);
> - dev->platform_data = pdata;
> drvdata->pdev = pdev;
> ret = arm_trbe_probe_irq(pdev, drvdata);
> if (ret)
On 2023/8/16 22:10, Suzuki K Poulose wrote:
> Coresight TRBE driver shares a single platform data (which is empty btw).
> However, with the commit 4e8fe7e5c3a5
> ("coresight: Store pointers to connections rather than an array of them")
> the coresight core would free up the pdata, resulting in multiple attempts
> to free the same pdata for TRBE instances. Fix this by allocating a pdata per
> coresight_device.
>
> Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
> Link: https://lore.kernel.org/r/[email protected]
> Reported-by: Junhao He <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Signed-off-by: Suzuki K Poulose <[email protected]>
Test-by: Junhao He <[email protected]>
> ---
> drivers/hwtracing/coresight/coresight-trbe.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 025f70adee47..d3d34a833f01 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -1255,10 +1255,13 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp
> if (!desc.name)
> goto cpu_clear;
>
> + desc.pdata = coresight_get_platform_data(dev);
> + if (IS_ERR(desc.pdata))
> + goto cpu_clear;
> +
> desc.type = CORESIGHT_DEV_TYPE_SINK;
> desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM;
> desc.ops = &arm_trbe_cs_ops;
> - desc.pdata = dev_get_platdata(dev);
> desc.groups = arm_trbe_groups;
> desc.dev = dev;
> trbe_csdev = coresight_register(&desc);
> @@ -1482,7 +1485,6 @@ static void arm_trbe_remove_irq(struct trbe_drvdata *drvdata)
>
> static int arm_trbe_device_probe(struct platform_device *pdev)
> {
> - struct coresight_platform_data *pdata;
> struct trbe_drvdata *drvdata;
> struct device *dev = &pdev->dev;
> int ret;
> @@ -1497,12 +1499,7 @@ static int arm_trbe_device_probe(struct platform_device *pdev)
> if (!drvdata)
> return -ENOMEM;
>
> - pdata = coresight_get_platform_data(dev);
> - if (IS_ERR(pdata))
> - return PTR_ERR(pdata);
> -
> dev_set_drvdata(dev, drvdata);
> - dev->platform_data = pdata;
> drvdata->pdev = pdev;
> ret = arm_trbe_probe_irq(pdev, drvdata);
> if (ret)
On 8/17/23 15:31, Suzuki K Poulose wrote:
> On 17/08/2023 10:24, James Clark wrote:
>>
>>
>> On 17/08/2023 07:37, Anshuman Khandual wrote:
>>> Hi Suzuki,
>>>
>>> Seems like this patch is going to conflict with the below proposed change
>>>
>>> https://lore.kernel.org/all/[email protected]/
>>>
>>> Please let me know how should we resolve this conflict.
>>
>> We could merge them both, with the fixes: one first, just to acknowledge
>> that there was a problem. But I suppose your one will have to be rebased
>> on top.
>>
>>>
>>> On 8/16/23 19:40, Suzuki K Poulose wrote:
>>>> Coresight TRBE driver shares a single platform data (which is empty btw).
>>>> However, with the commit 4e8fe7e5c3a5
>>>> ("coresight: Store pointers to connections rather than an array of them")
>>>> the coresight core would free up the pdata, resulting in multiple attempts
>>>> to free the same pdata for TRBE instances. Fix this by allocating a pdata per
>>>> coresight_device.
>>>>
>>>> Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
>>>
>>> The above mentioned commit i.e 4e8fe7e5c3a5 seems to be a more recent one which
>>> has triggered this problem. But would the problem be still there without that ?
>>> Else 'Fixes:' tag would need changing.
>>>
>>
>> Yes I think the fixes tag should point to 4e8fe7e5c3a5.
>
> Agreed, I will change the fixes tag and push this.
In the first patch, the last hunk might not be required to fix the
IPI problem and in fact might be bit problematic as well. Besides,
could you please hold off pushing this change into coresight tree
for some time ?