[dpdk-dev,3/6] eal: remove log level from internal config
Checks
Commit Message
This field is only used in the initialization phase. Remove it since the
global log level can also be retrieved using a public API:
rte_log_get_global_level().
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_eal/bsdapp/eal/eal.c | 2 --
lib/librte_eal/common/eal_common_log.c | 6 ++++++
lib/librte_eal/common/eal_common_options.c | 11 ++---------
lib/librte_eal/common/eal_internal_cfg.h | 1 -
lib/librte_eal/linuxapp/eal/eal.c | 2 --
5 files changed, 8 insertions(+), 14 deletions(-)
Comments
On 4/18/2017 3:22 PM, Olivier Matz wrote:
> This field is only used in the initialization phase. Remove it since the
> global log level can also be retrieved using a public API:
> rte_log_get_global_level().
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
<...>
> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index dd4d30ca7..7d13cc026 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -244,6 +244,12 @@ RTE_INIT(rte_log_init);
> static void
> rte_log_init(void)
> {
> +#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
Why ">=" (I aware previous one is also like this :), setting global
config option to RTE_LOG_DEBUG cause a INFO level log...
> + rte_log_set_global_level(RTE_LOG_INFO);
> +#else
> + rte_log_set_global_level(RTE_LOG_LEVEL);
> +#endif
Hi Ferruh,
On Tue, 18 Apr 2017 16:00:45 +0100, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 4/18/2017 3:22 PM, Olivier Matz wrote:
> > This field is only used in the initialization phase. Remove it since the
> > global log level can also be retrieved using a public API:
> > rte_log_get_global_level().
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>
> <...>
>
> > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> > index dd4d30ca7..7d13cc026 100644
> > --- a/lib/librte_eal/common/eal_common_log.c
> > +++ b/lib/librte_eal/common/eal_common_log.c
> > @@ -244,6 +244,12 @@ RTE_INIT(rte_log_init);
> > static void
> > rte_log_init(void)
> > {
> > +#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
>
> Why ">=" (I aware previous one is also like this :), setting global
> config option to RTE_LOG_DEBUG cause a INFO level log...
>
> > + rte_log_set_global_level(RTE_LOG_INFO);
> > +#else
> > + rte_log_set_global_level(RTE_LOG_LEVEL);
> > +#endif
>
From what I see, in the previous commit 9b9d7caa8414 ("log: increase default
level to info"), some code was added to prevent to set the default log
level to DEBUG, because it has an impact on performance.
But since commit 5d8f0baf69ea ("log: do not drop debug logs at compile time"),
the logs that impact performance should use RTE_LOG_DP().
I think this can be removed, and changed to:
rte_log_set_global_level(RTE_LOG_LEVEL);
I did not do it in this patchset because I think it can wait next
version.
On 4/18/2017 4:26 PM, Olivier MATZ wrote:
> Hi Ferruh,
>
> On Tue, 18 Apr 2017 16:00:45 +0100, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>> On 4/18/2017 3:22 PM, Olivier Matz wrote:
>>> This field is only used in the initialization phase. Remove it since the
>>> global log level can also be retrieved using a public API:
>>> rte_log_get_global_level().
>>>
>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>
>> <...>
>>
>>> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
>>> index dd4d30ca7..7d13cc026 100644
>>> --- a/lib/librte_eal/common/eal_common_log.c
>>> +++ b/lib/librte_eal/common/eal_common_log.c
>>> @@ -244,6 +244,12 @@ RTE_INIT(rte_log_init);
>>> static void
>>> rte_log_init(void)
>>> {
>>> +#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
>>
>> Why ">=" (I aware previous one is also like this :), setting global
>> config option to RTE_LOG_DEBUG cause a INFO level log...
>>
>>> + rte_log_set_global_level(RTE_LOG_INFO);
>>> +#else
>>> + rte_log_set_global_level(RTE_LOG_LEVEL);
>>> +#endif
>>
>
> From what I see, in the previous commit 9b9d7caa8414 ("log: increase default
> level to info"), some code was added to prevent to set the default log
> level to DEBUG, because it has an impact on performance.
>
> But since commit 5d8f0baf69ea ("log: do not drop debug logs at compile time"),
> the logs that impact performance should use RTE_LOG_DP().
>
> I think this can be removed, and changed to:
>
> rte_log_set_global_level(RTE_LOG_LEVEL);
>
>
> I did not do it in this patchset because I think it can wait next
> version.
>
Ok, thanks for clarification.
@@ -323,8 +323,6 @@ eal_log_level_parse(int argc, char **argv)
optind = 1;
optreset = 1;
- rte_log_set_global_level(internal_config.log_level);
-
while ((opt = getopt_long(argc, argvopt, eal_short_options,
eal_long_options, &option_index)) != EOF) {
@@ -244,6 +244,12 @@ RTE_INIT(rte_log_init);
static void
rte_log_init(void)
{
+#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
+ rte_log_set_global_level(RTE_LOG_INFO);
+#else
+ rte_log_set_global_level(RTE_LOG_LEVEL);
+#endif
+
rte_logs.dynamic_types = calloc(RTE_LOGTYPE_FIRST_EXT_ID,
sizeof(struct rte_log_dynamic_type));
if (rte_logs.dynamic_types == NULL)
@@ -148,12 +148,6 @@ eal_reset_internal_config(struct internal_config *internal_cfg)
internal_cfg->base_virtaddr = 0;
internal_cfg->syslog_facility = LOG_DAEMON;
- /* default value from build option */
-#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
- internal_cfg->log_level = RTE_LOG_INFO;
-#else
- internal_cfg->log_level = RTE_LOG_LEVEL;
-#endif
internal_cfg->xen_dom0_support = 0;
@@ -739,7 +733,7 @@ eal_parse_syslog(const char *facility, struct internal_config *conf)
}
static int
-eal_parse_log_level(const char *arg, struct internal_config *conf)
+eal_parse_log_level(const char *arg)
{
char *end, *str, *type, *level;
unsigned long tmp;
@@ -772,7 +766,6 @@ eal_parse_log_level(const char *arg, struct internal_config *conf)
type, tmp);
if (type == NULL) {
- conf->log_level = tmp;
rte_log_set_global_level(tmp);
} else if (rte_log_set_level_regexp(type, tmp) < 0) {
printf("cannot set log level %s,%lu\n",
@@ -926,7 +919,7 @@ eal_parse_common_option(int opt, const char *optarg,
break;
case OPT_LOG_LEVEL_NUM: {
- if (eal_parse_log_level(optarg, conf) < 0) {
+ if (eal_parse_log_level(optarg) < 0) {
RTE_LOG(ERR, EAL,
"invalid parameters for --"
OPT_LOG_LEVEL "\n");
@@ -78,7 +78,6 @@ struct internal_config {
volatile uint64_t socket_mem[RTE_MAX_NUMA_NODES]; /**< amount of memory per socket */
uintptr_t base_virtaddr; /**< base address to try and reserve memory from */
volatile int syslog_facility; /**< facility passed to openlog() */
- volatile uint32_t log_level; /**< default log level */
/** default interrupt mode for VFIO */
volatile enum rte_intr_mode vfio_intr_mode;
const char *hugefile_prefix; /**< the base filename of hugetlbfs files */
@@ -492,8 +492,6 @@ eal_log_level_parse(int argc, char **argv)
argvopt = argv;
optind = 1;
- rte_log_set_global_level(internal_config.log_level);
-
while ((opt = getopt_long(argc, argvopt, eal_short_options,
eal_long_options, &option_index)) != EOF) {