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] [thread-next>] [day] [month] [year] [list]
Message-ID: <d7960d95-d076-4c0d-929c-421480f330d4@oss.qualcomm.com>
Date: Wed, 6 Aug 2025 08:45:44 +0800
From: Jie Gan <jie.gan@....qualcomm.com>
To: Mike Leach <mike.leach@...aro.org>, Jie Gan <jie.gan@....qualcomm.com>
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
        James Clark <james.clark@...aro.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Tingwei Zhang <tingwei.zhang@....qualcomm.com>,
        Jinlong Mao <jinlong.mao@....qualcomm.com>, coresight@...ts.linaro.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 04/10] coresight: tmc: add create/delete functions for
 etr_buf_node



On 8/5/2025 6:27 PM, Mike Leach wrote:
> Hi,
> 
> On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan@....qualcomm.com> wrote:
>>
>> Create and insert or remove the etr_buf_node to/from the etr_buf_list.
>>
>> Signed-off-by: Jie Gan <jie.gan@....qualcomm.com>
>> ---
>>   .../hwtracing/coresight/coresight-tmc-etr.c   | 65 +++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tmc.h   |  2 +
>>   2 files changed, 67 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index b07fcdb3fe1a..e8ecb3e087ab 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1909,6 +1909,71 @@ const struct coresight_ops tmc_etr_cs_ops = {
>>          .panic_ops      = &tmc_etr_sync_ops,
>>   };
>>
>> +/**
>> + * tmc_clean_etr_buf_list - clean the etr_buf_list.
>> + * @drvdata:   driver data of the TMC device.
>> + *
>> + * Remove the allocated node from the list and free the extra buffer.
>> + */
>> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata)
>> +{
>> +       struct etr_buf_node *nd, *next;
>> +
>> +       list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) {
>> +               if (nd->sysfs_buf == drvdata->sysfs_buf) {
>> +                       list_del(&nd->node);
>> +                       kfree(nd);
>> +               } else {
>> +                       /* Free allocated buffers which are not utilized by ETR */
>> +                       list_del(&nd->node);
>> +                       tmc_free_etr_buf(nd->sysfs_buf);
>> +                       nd->sysfs_buf = NULL;
>> +                       kfree(nd);
>> +               }
>> +       }
>> +}
>> +EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list);
>> +
>> +/**
>> + * tmc_create_etr_buf_node - create a node to store the alloc_buf and
>> + * insert the node to the etr_buf_list. Create a new buffer if the
>> + * alloc_buf is NULL.
>> + * @drvdata:   driver data of the TMC device.
>> + * @alloc_buf: the buffer that is inserted to the list.
>> + *
>> + * Return 0 upon success and return the error number if fail.
>> + */
>> +int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf)
> 
> This list handle function pair look a little asymmetric. Is it not
> possible to change this to tmc_create_etr_buf_list(struct tmc_drvdata
> *drvdata, int num_nodes)
> so that one function creates all the nodes, and another destroys them.
> 
> In the logic that decides between using multi buffer or single buffer
> you can then use  a construct such as:
> 
> if (single_buffer)
>        drvdata->sysfs_buf = <alloc sysfs buffer>
> else {
>       tmc_create_etr_buf_list(drvdata, 2);
>       <switch in avail buffer to drvdata->sysfs_buf
> }

The lsit handle function pair definitely looks a little bit asymmetric. 
I will consider the suggestion in next version.

Thanks,
Jie

> 
>> +{
>> +       struct etr_buf_node *sysfs_buf_node;
>> +       struct etr_buf *sysfs_buf;
>> +
>> +       if (!alloc_buf) {
>> +               sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL);
>> +               if (IS_ERR(sysfs_buf))
>> +                       return PTR_ERR(sysfs_buf);
>> +       } else
>> +               sysfs_buf = alloc_buf;
>> +
>> +       sysfs_buf_node = kzalloc(sizeof(struct etr_buf_node), GFP_KERNEL);
>> +       if (IS_ERR(sysfs_buf_node)) {
>> +               if (!alloc_buf)
>> +                       tmc_free_etr_buf(sysfs_buf);
>> +               return PTR_ERR(sysfs_buf_node);
>> +       }
>> +
>> +       sysfs_buf_node->sysfs_buf = sysfs_buf;
>> +       sysfs_buf_node->reading = false;
>> +       if (!alloc_buf)
>> +               sysfs_buf_node->is_free = true;
>> +       else
>> +               sysfs_buf_node->is_free = false;
>> +       list_add(&sysfs_buf_node->node, &drvdata->etr_buf_list);
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(tmc_create_etr_buf_node);
>> +
>>   int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>>   {
>>          int ret = 0;
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
>> index 52ee5f8efe8c..3cb8ba9f88f5 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
>> @@ -461,5 +461,7 @@ void tmc_etr_remove_catu_ops(void);
>>   struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
>>                                     enum cs_mode mode, void *data);
>>   extern const struct attribute_group coresight_etr_group;
>> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata);
>> +int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf);
>>
>>   #endif
>> --
>> 2.34.1
>>
> 
> Regards
> 
> Mike


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ