From: hejunhao <hejunhao3@huawei.com>
To: Suzuki K Poulose <suzuki.poulose@arm.com>,
<mike.leach@linaro.org>, <leo.yan@linaro.org>,
<anshuman.khandual@arm.com>, <jonathan.cameron@huawei.com>,
James Clark <james.clark@arm.com>
Cc: <coresight@lists.linaro.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <linuxarm@huawei.com>,
<yangyicong@huawei.com>, <prime.zeng@hisilicon.com>
Subject: Re: [PATCH 2/2] coresight: core: Fix multiple free TRBE platform data resource
Date: Tue, 15 Aug 2023 19:38:17 +0800 [thread overview]
Message-ID: <68f16bb1-53ed-b8b9-812e-6d3be40a5bde@huawei.com> (raw)
In-Reply-To: <2b69bd4e-5ef4-16c7-f908-7c70187e12b6@arm.com>
Hi, Suzuki
On 2023/8/15 6:47, Suzuki K Poulose wrote:
> + James Clark
>
> On 14/08/2023 10:38, Junhao He wrote:
>> Current the TRBE driver supports matching TRBE platform device through
>> id_table. The ACPI created a dummy TRBE platform device inside
>> drivers/perf/arm_pmu_acpi.c. So the TRBE platform driver will probe only
>> once and allocate just one TRBE platform data resource.
>>
>> If the system supports the TRBE feature, Each CPU in the systems can
>> have at least one TRBE present, and the coresight_unregister gets called
>> multiple times, once for each of them.
>> Therefore, when unregister TRBE coresight devices, the TRBE platform
>> data
>> resource will multiple free in function coresight_unregister.
>>
>> root@localhost:# insmod coresight-trbe.ko
>> root@localhost:# rmmod coresight-trbe.ko
>> [ 423.455932] ------------[ cut here ]------------
>> [ 423.461987] WARNING: CPU: 1 PID: 0 at drivers/base/devres.c:1064
>> devm_kfree+0x88/0x98
>> [ 423.483821] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G
>> O 6.5.0-rc4+ #1
>> [ 423.505842] pstate: 614000c9 (nZCv daIF +PAN -UAO -TCO +DIT -SSBS
>> BTYPE=--)
>> ...
>> [ 423.601301] Call trace:
>> [ 423.604202] devm_kfree+0x88/0x98
>> [ 423.608369] coresight_release_platform_data+0xb8/0xe0 [coresight]
>> [ 423.616589] coresight_unregister+0x120/0x170 [coresight]
>> [ 423.623533] arm_trbe_remove_coresight_cpu+0x70/0xa0 [coresight_trbe]
>> [ 423.631082] __flush_smp_call_function_queue+0x1e4/0x4e0
>> [ 423.637471] generic_smp_call_function_single_interrupt+0x1c/0x30
>> [ 423.644796] ipi_handler+0x90/0x278
>> [ 423.648992] handle_percpu_devid_irq+0x90/0x250
>> [ 423.654636] generic_handle_domain_irq+0x34/0x58
>> [ 423.659786] gic_handle_irq+0x12c/0x270
>> [ 423.664039] call_on_irq_stack+0x24/0x30
>> [ 423.668452] do_interrupt_handler+0x88/0x98
>> [ 423.673027] el1_interrupt+0x48/0xe8
>> [ 423.677413] el1h_64_irq_handler+0x18/0x28
>> [ 423.681781] el1h_64_irq+0x78/0x80
>> [ 423.685550] default_idle_call+0x5c/0x180
>> [ 423.689855] do_idle+0x25c/0x2c0
>> [ 423.694196] cpu_startup_entry+0x2c/0x40
>> [ 423.698373] secondary_start_kernel+0x144/0x188
>> [ 423.703920] __secondary_switched+0xb8/0xc0
>> [ 423.708972] ---[ end trace 0000000000000000 ]---
>> [ 423.729209] ------------[ cut here ]------------
>> ...
>> [ 423.735217] WARNING: CPU: 2 PID: 40 at drivers/base/devres.c:1064
>> devm_kfree+0x88/0x98
>> ...
>> [ 424.012385] WARNING: CPU: 3 PID: 0 at drivers/base/devres.c:1064
>> devm_kfree+0x88/0x98
>> ...
>>
>> This patch does the following:
>> 1.TRBE coresight devices do not need regular connections information, We
>> can free connections resource when the nr_conns is valid.
>> 2.And we can ignore the free platform data resource, it will be
>> automatically free in platform_driver_unregister().
>
> Do we need a Fixes tag here ?
Yes, I will do that.
>>
>> Signed-off-by: Junhao He <hejunhao3@huawei.com>
>> ---
>> drivers/hwtracing/coresight/coresight-core.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>> b/drivers/hwtracing/coresight/coresight-core.c
>> index 118fcf27854d..c6f7889d1b4d 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -1555,9 +1555,10 @@ void coresight_release_platform_data(struct
>> coresight_device *csdev,
>> conns[i]->dest_fwnode = NULL;
>> devm_kfree(dev, conns[i]);
>> }
>> - devm_kfree(dev, pdata->out_conns);
>> - devm_kfree(dev, pdata->in_conns);
>> - devm_kfree(dev, pdata);
>> + if (pdata->nr_outconns)
>> + devm_kfree(dev, pdata->out_conns);
>> + if (pdata->nr_inconns)
>> + devm_kfree(dev, pdata->in_conns);
>
> These allocations are made on the parent device and that
> may never get unregistered (e.g., AMBA device, platform device,
> stay forever, even when the "coresight" modules are unloaded).
> Thus the memory will be left unused, literally leaking.
> This specific devm_kfree() was added to fix that. May be we should fix
> this in the TRBE driver to use separate pdata for the TRBE device
> instances.
>
> Suzuki
If we fix this with minimal changes, I think it is possible to add a check
and not free pdata if it is TRBE?
if (csdev->subtype.sink_subtype !=
CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM)
devm_kfree(dev, pdata);
Then free pdata in the end of arm_trbe_remove_coresight().
>
>> if (csdev)
>> coresight_remove_conns_sysfs_group(csdev);
>> }
>
>
> .
>
next prev parent reply other threads:[~2023-08-15 11:39 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-14 9:38 [PATCH 0/2] Fix some issues with TRBE building as a module Junhao He
2023-08-14 9:38 ` [PATCH 1/2] coresight: trbe: Fix TRBE potential sleep in atomic context Junhao He
2023-08-14 10:34 ` Suzuki K Poulose
2023-08-14 13:32 ` hejunhao
2023-08-14 22:57 ` Suzuki K Poulose
2023-08-15 11:40 ` hejunhao
2023-08-14 9:38 ` [PATCH 2/2] coresight: core: Fix multiple free TRBE platform data resource Junhao He
2023-08-14 22:47 ` Suzuki K Poulose
2023-08-15 11:38 ` hejunhao [this message]
2023-08-16 13:13 ` Suzuki K Poulose
2023-08-16 13:58 ` Suzuki K Poulose
2023-08-16 14:10 ` [PATCH v2 1/2] coresight: trbe: Fix TRBE potential sleep in atomic context Suzuki K Poulose
2023-08-16 14:10 ` [PATCH 2/2] coresight: trbe: Allocate platform data per device Suzuki K Poulose
2023-08-17 6:37 ` Anshuman Khandual
2023-08-17 9:24 ` James Clark
2023-08-17 10:01 ` Suzuki K Poulose
2023-08-17 10:16 ` Anshuman Khandual
2023-08-17 10:33 ` Suzuki K Poulose
2023-08-17 10:01 ` Suzuki K Poulose
2023-08-17 8:47 ` hejunhao
2023-08-17 7:13 ` [PATCH v2 1/2] coresight: trbe: Fix TRBE potential sleep in atomic context Anshuman Khandual
2023-08-17 8:41 ` hejunhao
2023-08-17 9:57 ` James Clark
2023-08-17 9:59 ` Suzuki K Poulose
2023-08-17 6:18 ` [PATCH 0/2] Fix some issues with TRBE building as a module Anshuman Khandual
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=68f16bb1-53ed-b8b9-812e-6d3be40a5bde@huawei.com \
--to=hejunhao3@huawei.com \
--cc=anshuman.khandual@arm.com \
--cc=coresight@lists.linaro.org \
--cc=james.clark@arm.com \
--cc=jonathan.cameron@huawei.com \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=mike.leach@linaro.org \
--cc=prime.zeng@hisilicon.com \
--cc=suzuki.poulose@arm.com \
--cc=yangyicong@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).