Re: [PATCH v2 3/3] baseband/acc: add internal logging

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi Nicolas,

On 2/7/25 10:52 AM, Maxime Coquelin wrote:
> Hi Nicolas,
> 
> On 1/24/25 7:00 PM, Stephen Hemminger wrote:
>> On Fri, 24 Jan 2025 17:52:43 +0000
>> "Chautru, Nicolas" <nicolas.chautru@intel.com> wrote:
>>
>>> Hi Stephen,
>>>
>>> The commit message may be misleading, the logging interface doesn't 
>>> change here. Note also that I reuse existing trace point framework 
>>> for some of the error logging when relevant (see previous commit).
>>> Here the scope for that buffer is more limited, not a new logging 
>>> method really (the commit message is misleading).
>>> The queue_ops_dump() already provides api to dump device specific 
>>> information related to queue into file (logging in real time is not 
>>> an option) based on information already in PMD memory.
>>> This new buffer is purely there to add storage for the string out of 
>>> rte_bbdev_ops_param_string() for failed operation on that queue, so 
>>> that extend that capture as this info is not stored by PMD.
>>> The name of the buffer could be renamed probably, or I could store 
>>> copy of the actual operation instead of the string in case that makes 
>>> a difference for you.
>>>
>>> I guess it would possible to move this to trace point but I thought 
>>> it would be quite convoluted. That information would fits nicely in 
>>> the queue dump capture, and this would require adding trace point for 
>>> each operation type (I don't believe it can manage arbitrary string) 
>>> and would be a bit of an unconventional use of trace point.
>>>
>>> Any thought?
> 
> I think the introduction of trace points in patch 2 is a good addition,
> and could be extended further to not just emit a simple string but also
> the necessary values to enable debugging (basically what you write in
> the buffers).
> 
> It would have the advantage of having the different traces to be
> synchronized (bbdev and acc ones), and also would have less performance
> impact.

What do you think of this proposal?
how do we proceed for v25.03?

Thanks,
Maxime

>>> Thanks
>>> Nic
>>>
>>>
>>>> -----Original Message-----
>>>> From: Stephen Hemminger <stephen@networkplumber.org>
>>>> Sent: Thursday, January 23, 2025 3:24 PM
>>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>
>>>> Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
>>>> hemant.agrawal@nxp.com; Vargas, Hernan <hernan.vargas@intel.com>
>>>> Subject: Re: [PATCH v2 3/3] baseband/acc: add internal logging
>>>>
>>>> On Thu, 23 Jan 2025 14:55:19 -0800
>>>> Nicolas Chautru <nicolas.chautru@intel.com> wrote:
>>>>> Adds internal buffer for more flexible logging.
>>>>>
>>>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>>>
>>>> Inventing another device specific error log seems like a short sighted
>>>> concept.
>>>> Why doesn't existing DPDK logging work well enough?
>>>
>>
>> My feedback is that why can't you just use DEBUG logging for this.
> 
> Or indeed could use the existing logging mechanism.
> 
>>
> 
> Thanks,
> Maxime