From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Stephen Hemminger <stephen@networkplumber.org>,
"Chautru, Nicolas" <nicolas.chautru@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"Vargas, Hernan" <hernan.vargas@intel.com>
Subject: Re: [PATCH v2 3/3] baseband/acc: add internal logging
Date: Fri, 7 Feb 2025 10:52:51 +0100 [thread overview]
Message-ID: <0df7deea-a6f9-4078-9dee-de42a1e647f8@redhat.com> (raw)
In-Reply-To: <20250124100058.5b4de64f@hermes.local>
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.
>> 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
prev parent reply other threads:[~2025-02-07 9:53 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-23 22:55 [PATCH v2 0/3] bbdev: trace point and logging Nicolas Chautru
2025-01-23 22:55 ` [PATCH v2 1/3] bbdev: add trace point Nicolas Chautru
2025-02-07 8:40 ` Maxime Coquelin
2025-01-23 22:55 ` [PATCH v2 2/3] baseband/acc: " Nicolas Chautru
2025-01-23 22:55 ` [PATCH v2 3/3] baseband/acc: add internal logging Nicolas Chautru
2025-01-23 23:24 ` Stephen Hemminger
2025-01-24 17:52 ` Chautru, Nicolas
2025-01-24 18:00 ` Stephen Hemminger
2025-01-24 21:21 ` Chautru, Nicolas
2025-02-07 9:52 ` Maxime Coquelin [this message]
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=0df7deea-a6f9-4078-9dee-de42a1e647f8@redhat.com \
--to=maxime.coquelin@redhat.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=hernan.vargas@intel.com \
--cc=nicolas.chautru@intel.com \
--cc=stephen@networkplumber.org \
/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).