DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Pavan Nikhilesh Bhagavatula <pbhagavatula@caviumnetworks.com>,
	olivier.matz@6wind.com, thomas@monjalon.net
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 2/2] eal: fix dynamic logs failing to print
Date: Mon, 20 Nov 2017 10:40:29 -0800	[thread overview]
Message-ID: <9e17df24-9b52-620f-b299-fdc42a1083c4@intel.com> (raw)
In-Reply-To: <20171117084722.by7sdxdoimkwdqpg@Pavan-LT>

On 11/17/2017 12:47 AM, Pavan Nikhilesh Bhagavatula wrote:
> On Thu, Nov 16, 2017 at 11:48:14AM -0800, Ferruh Yigit wrote:
>> On 11/16/2017 2:31 AM, Pavan Nikhilesh wrote:
>>> Dynamic logs fail to print if the global log level is less than dynamic
>>> loglevel. Example if the global log level is set to INFO and dynamic log
>>> level for a specific module is set to DEBUG then the log fails to print.
>>
>> What is the purpose of the global log level value?
>>
> 
> As per my understanding, If we want to filter out debug logs from all other
> modules except a specific module using --log-level="<regex>,<level>" the logs
> would fail to print if the global log level is lower than the specified level.

Yes.

> 
> If during EAL init we want to filter out all the logs except a certain pmd's
> log we can specify --log-level="pmd\..*,<level>" without modifying the global
> log level. Also, this wouldnt break the previous functionality of global log
> level.

But this overwrites the global and makes it useless.

Lets say global log level is set to "critical" (3), if module set something
bigger like "info" (7) that will set the limit, so module will be controlling
and what is the point of having global level?

It can be an option remove global level completely and let modules control what
to log, but I believe it can be useful to have a global control.


What about having a verbose global and limited module log levels:
global default: debug (8)
modules default: info (7)

so if you want get verbose log from specific module:
--log-level="<regex>,8"

for globally less verbose log:
--log-level=3

But this makes hard to enable verbose log from everything, although not sure how
practical is this. For this one it can be possible to have a new API to set
modules other than given one, like:
--log-level="eal,-4"  --> set level of all modules except "eal" to "error" (4)


Also I believe it is possible to remove RTE_LOG_LEVEL config option completely
since dynamic debug exist now. Previously it was good to have because it was
used to compile out debug functions in data path, now there is
"RTE_LOG_DP_LEVEL" for that purpose, so it can be possible to remove
RTE_LOG_LEVEL and in rte_log_init() set to DEBUG by default if you agree with
above approach.



And, since you are touching these files :),
for better regex support, it may be good to have log types hierarchical, like
"lib.mbuf", "lib.mempool", "lib.eal.malloc", as done in member library but with
"lib" prefix instead of "librte", if this makes sense and you have time would
you mind sending a patch for this as well?


Thanks,
ferruh

> 
>>> Check modules log level before checking the global log level.
>>>
>>> Fixes: c1b5fa94a46f ("eal: support dynamic log types")
>>>
>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
>>
>> <...>
>>

  reply	other threads:[~2017-11-20 18:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-16 10:31 [dpdk-dev] [PATCH 1/2] eal: fix legacy dynamic logs defaulting to debug Pavan Nikhilesh
2017-11-16 10:31 ` [dpdk-dev] [PATCH 2/2] eal: fix dynamic logs failing to print Pavan Nikhilesh
2017-11-16 19:48   ` Ferruh Yigit
2017-11-17  8:47     ` Pavan Nikhilesh Bhagavatula
2017-11-20 18:40       ` Ferruh Yigit [this message]
2017-11-21  8:30         ` Pavan Nikhilesh Bhagavatula
2017-11-21 10:55         ` Thomas Monjalon
2017-11-21 11:14           ` Pavan Nikhilesh Bhagavatula
2017-11-21 13:30             ` Thomas Monjalon
2017-11-21 14:04               ` Pavan Nikhilesh Bhagavatula
2017-11-21 18:10                 ` Ferruh Yigit

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=9e17df24-9b52-620f-b299-fdc42a1083c4@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    --cc=pbhagavatula@caviumnetworks.com \
    --cc=thomas@monjalon.net \
    /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).