Re: [dpdk-dev] [PATCH v4 00/33] DPDK Trace support

[Thomas Monjalon <thomas@monjalon.net>]

09/04/2020 18:34, Jerin Jacob:
> On Thu, Apr 9, 2020 at 9:30 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > 09/04/2020 17:36, Jerin Jacob:
> > > On Thu, Apr 9, 2020 at 7:30 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > As discussed somewhere else, please do not introduce global level
> > > > in rte_trace. I think it is useless. If we need to change the level
> > > > of all trace types, we can just use a wildcard (globbing or regexp).
> > >
> > > Currently, In the log library, when EAL command-line argument
> > > specifies the "--log-level=val" it, will call
> > > the  rte_log_global_level_set(val)
> > >
> > > Is the suggestion to make rte_log_global_level_set() as an internal
> > > EAL API or remove that feature?
> >
> > Completely remove global level.
> > Why would we need 2 levels of level?
> >
> > > If we remove, then I dont know, how we can map --log-level or
> > > --trace-level EAL command-line argument.
> >
> > They are setting levels with regex or globbing.
> > --log-level supports 3 syntaxes today:
> >         - int (global level)
> >         - globbing:int
> >         - regex,int
> 
> Here is my understanding.
> 
> IMO, Actual Syntax is
>          - int (global level)
>          - globbing: int (global level)
>          - regex: int (global level)

The level apply to the logs matching the pattern (globbing or regex)
so I don't understand why you call it "global".

> i.e
> 
> 1) Each log/trace has a  "level"
> 2) There is a global level. See [1]
> 3) The trace will be emitted when
> a) When it is enabled(not applicable for log as it is always enabled)
> AND
> b) it is less than the global level.

When it is less than both global and logtype level.
See rte_log_can_log().

> The global level will be useful to control what category of trace
> "level" needs to enable.
> I think, it is useful to trace only DEBUG or CRITICAL events, Any
> reason for thinking, it is not.
> 
> 
> [1]
> /**
>  * Set the global log level.
>  *
>  * After this call, logs with a level lower or equal than the level
>  * passed as argument will be displayed.
>  *
>  * @param level
>  *   Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
>  */
> void rte_log_set_global_level(uint32_t level);

rte_log_can_log(uint32_t logtype, uint32_t level)
{
    int log_level;

    if (level > rte_log_get_global_level())
        return false;

    log_level = rte_log_get_level(logtype);
    if (log_level < 0)
        return false;

    if (level > (uint32_t)log_level)
        return false;

    return true;
}

The global level is just disabling some logs even if it is enabled
in the logtype level.
It only makes usage complicate.
We should consider only logtype levels.

Currently, the global log level is disabled by default by setting
the global level to DEBUG:

RTE_INIT_PRIO(rte_log_init, LOG)
{
[...]
    rte_log_set_global_level(RTE_LOG_DEBUG);

It was done by Pavan 2 years ago.