linux-kernel - Re: [PATCH] coresight: Add a KUnit test for coresight_find_default

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <67c484d7-6929-4a65-b89c-f3fb85682028@linaro.org>
Date: Wed, 5 Mar 2025 13:52:46 +0000
From: James Clark <james.clark@...aro.org>
To: Leo Yan <leo.yan@....com>
Cc: coresight@...ts.linaro.org, scclevenger@...amperecomputing.com,
 yeoreum.yun@....com, Mike Leach <mike.leach@...aro.org>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] coresight: Add a KUnit test for
 coresight_find_default_sink()



On 05/03/2025 11:42 am, Leo Yan wrote:
> On Tue, Dec 17, 2024 at 05:11:31PM +0000, James Clark wrote:
>>
>> Add a test to confirm that default sink selection skips over an ETF
>> and returns an ETR even if it's further away.
>>
>> This also makes it easier to add new unit tests in the future.
>>
>> Signed-off-by: James Clark <james.clark@...aro.org>
>> ---
>>   drivers/hwtracing/coresight/Kconfig           |  9 ++
>>   drivers/hwtracing/coresight/Makefile          |  1 +
>>   drivers/hwtracing/coresight/coresight-core.c  |  1 +
>>   .../coresight/coresight-kunit-tests.c         | 90 +++++++++++++++++++
>>   4 files changed, 101 insertions(+)
>>   create mode 100644 drivers/hwtracing/coresight/coresight-kunit-tests.c
>>
>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
>> index 06f0a7594169..056d04bc540a 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -247,4 +247,13 @@ config CORESIGHT_DUMMY
>>
>>            To compile this driver as a module, choose M here: the module will be
>>            called coresight-dummy.
>> +
>> +config CORESIGHT_KUNIT_TESTS
>> +         tristate "Enable Coresight unit tests"
>> +         depends on KUNIT
>> +         default KUNIT_ALL_TESTS
>> +         help
>> +           Enable Coresight unit tests. Only useful for development and not
>> +           intended for production.
>> +
>>   endif
>> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
>> index 4ba478211b31..c170a41b3056 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -51,3 +51,4 @@ coresight-cti-y := coresight-cti-core.o       coresight-cti-platform.o \
>>                     coresight-cti-sysfs.o
>>   obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
>>   obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
>> +obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
>> index ea38ecf26fcb..ce63b68d5503 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -869,6 +869,7 @@ coresight_find_default_sink(struct coresight_device *csdev)
>>          }
>>          return csdev->def_sink;
>>   }
>> +EXPORT_SYMBOL_GPL(coresight_find_default_sink);
>>
>>   static int coresight_remove_sink_ref(struct device *dev, void *data)
>>   {
>> diff --git a/drivers/hwtracing/coresight/coresight-kunit-tests.c b/drivers/hwtracing/coresight/coresight-kunit-tests.c
>> new file mode 100644
>> index 000000000000..d022bacc8357
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-kunit-tests.c
>> @@ -0,0 +1,90 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <kunit/test.h>
>> +#include <kunit/device.h>
>> +#include <linux/coresight.h>
>> +
>> +#include "coresight-priv.h"
>> +
>> +static struct coresight_device *coresight_test_device(struct device *dev)
>> +{
>> +       struct coresight_device *csdev = devm_kcalloc(dev, 1,
>> +                                                    sizeof(struct coresight_device),
>> +                                                    GFP_KERNEL);
>> +       csdev->pdata = devm_kcalloc(dev, 1,
>> +                                  sizeof(struct coresight_platform_data),
>> +                                  GFP_KERNEL);
>> +       return csdev;
>> +}
>> +
>> +static void coresight_free_test_device(struct device *dev,
>> +                                      struct coresight_device *csdev)
>> +{
>> +       devm_kfree(dev, csdev->pdata->out_conns);
> 
> It will lead memory leaking if we don't release connections pointed by
> 'out_conns' pointer array.  Something like:
> 
>            for (i = 0; i < csdev->pdata->nr_outconns; i++)
>                 devm_kfree(dev, csdev->pdata->out_conns[i]);
>            devm_kfree(dev, csdev->pdata->out_conns);
> 
> Otherwise, it looks good to me.
> 
> Thanks,
> Leo
> 

Ah yeah, I'll just free the whole device instead of the individual 
kfrees then and let devm handle it.