Re: [dpdk-dev] [PATCH 2/2] eal: fix dynamic logs failing to print

[Ferruh Yigit <ferruh.yigit@intel.com>]

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>
>>
>> <...>
>>