[dpdk-dev,v4,11/17] log: fix the gap to support non-EAL thread
Commit Message
For those non-EAL thread, *_lcore_id* is invalid and probably larger than RTE_MAX_LCORE.
The patch adds the check and allows only EAL thread using EAL per thread log level and log type.
Others shares the global log level.
Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
lib/librte_eal/common/eal_common_log.c | 17 +++++++++++++++--
lib/librte_eal/common/include/rte_log.h | 5 +++++
2 files changed, 20 insertions(+), 2 deletions(-)
Comments
Hi,
On 02/02/2015 03:02 AM, Cunming Liang wrote:
> For those non-EAL thread, *_lcore_id* is invalid and probably larger than RTE_MAX_LCORE.
> The patch adds the check and allows only EAL thread using EAL per thread log level and log type.
> Others shares the global log level.
>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> ---
> lib/librte_eal/common/eal_common_log.c | 17 +++++++++++++++--
> lib/librte_eal/common/include/rte_log.h | 5 +++++
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index cf57619..e8dc94a 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -193,11 +193,20 @@ rte_set_log_type(uint32_t type, int enable)
> rte_logs.type &= (~type);
> }
>
> +/* Get global log type */
> +uint32_t
> +rte_get_log_type(void)
> +{
> + return rte_logs.type;
> +}
> +
> /* get the current loglevel for the message beeing processed */
> int rte_log_cur_msg_loglevel(void)
> {
> unsigned lcore_id;
> lcore_id = rte_lcore_id();
> + if (lcore_id >= RTE_MAX_LCORE)
> + return rte_get_log_level();
> return log_cur_msg[lcore_id].loglevel;
> }
>
> @@ -206,6 +215,8 @@ int rte_log_cur_msg_logtype(void)
> {
> unsigned lcore_id;
> lcore_id = rte_lcore_id();
> + if (lcore_id >= RTE_MAX_LCORE)
> + return rte_get_log_type();
> return log_cur_msg[lcore_id].logtype;
> }
>
> @@ -265,8 +276,10 @@ rte_vlog(__attribute__((unused)) uint32_t level,
>
> /* save loglevel and logtype in a global per-lcore variable */
> lcore_id = rte_lcore_id();
> - log_cur_msg[lcore_id].loglevel = level;
> - log_cur_msg[lcore_id].logtype = logtype;
> + if (lcore_id < RTE_MAX_LCORE) {
> + log_cur_msg[lcore_id].loglevel = level;
> + log_cur_msg[lcore_id].logtype = logtype;
> + }
>
> ret = vfprintf(f, format, ap);
> fflush(f);
> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
> index db1ea08..f83a0d9 100644
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -144,6 +144,11 @@ uint32_t rte_get_log_level(void);
> void rte_set_log_type(uint32_t type, int enable);
>
> /**
> + * Get the global log type.
> + */
> +uint32_t rte_get_log_type(void);
> +
> +/**
> * Get the current loglevel for the message being processed.
> *
> * Before calling the user-defined stream for logging, the log
>
Wouldn't it be better to change the variable:
static struct log_cur_msg log_cur_msg[RTE_MAX_LCORE];
into a pthread (tls) variable?
With your patch, the log level and log type are not saved for
non-EAL threads. If TLS were used, I think it would work in any case.
Regards,
Olivier
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Monday, February 09, 2015 4:01 AM
> To: Liang, Cunming; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 11/17] log: fix the gap to support non-EAL
> thread
>
> Hi,
>
> On 02/02/2015 03:02 AM, Cunming Liang wrote:
> > For those non-EAL thread, *_lcore_id* is invalid and probably larger than
> RTE_MAX_LCORE.
> > The patch adds the check and allows only EAL thread using EAL per thread log
> level and log type.
> > Others shares the global log level.
> >
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > ---
> > lib/librte_eal/common/eal_common_log.c | 17 +++++++++++++++--
> > lib/librte_eal/common/include/rte_log.h | 5 +++++
> > 2 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_log.c
> b/lib/librte_eal/common/eal_common_log.c
> > index cf57619..e8dc94a 100644
> > --- a/lib/librte_eal/common/eal_common_log.c
> > +++ b/lib/librte_eal/common/eal_common_log.c
> > @@ -193,11 +193,20 @@ rte_set_log_type(uint32_t type, int enable)
> > rte_logs.type &= (~type);
> > }
> >
> > +/* Get global log type */
> > +uint32_t
> > +rte_get_log_type(void)
> > +{
> > + return rte_logs.type;
> > +}
> > +
> > /* get the current loglevel for the message beeing processed */
> > int rte_log_cur_msg_loglevel(void)
> > {
> > unsigned lcore_id;
> > lcore_id = rte_lcore_id();
> > + if (lcore_id >= RTE_MAX_LCORE)
> > + return rte_get_log_level();
> > return log_cur_msg[lcore_id].loglevel;
> > }
> >
> > @@ -206,6 +215,8 @@ int rte_log_cur_msg_logtype(void)
> > {
> > unsigned lcore_id;
> > lcore_id = rte_lcore_id();
> > + if (lcore_id >= RTE_MAX_LCORE)
> > + return rte_get_log_type();
> > return log_cur_msg[lcore_id].logtype;
> > }
> >
> > @@ -265,8 +276,10 @@ rte_vlog(__attribute__((unused)) uint32_t level,
> >
> > /* save loglevel and logtype in a global per-lcore variable */
> > lcore_id = rte_lcore_id();
> > - log_cur_msg[lcore_id].loglevel = level;
> > - log_cur_msg[lcore_id].logtype = logtype;
> > + if (lcore_id < RTE_MAX_LCORE) {
> > + log_cur_msg[lcore_id].loglevel = level;
> > + log_cur_msg[lcore_id].logtype = logtype;
> > + }
> >
> > ret = vfprintf(f, format, ap);
> > fflush(f);
> > diff --git a/lib/librte_eal/common/include/rte_log.h
> b/lib/librte_eal/common/include/rte_log.h
> > index db1ea08..f83a0d9 100644
> > --- a/lib/librte_eal/common/include/rte_log.h
> > +++ b/lib/librte_eal/common/include/rte_log.h
> > @@ -144,6 +144,11 @@ uint32_t rte_get_log_level(void);
> > void rte_set_log_type(uint32_t type, int enable);
> >
> > /**
> > + * Get the global log type.
> > + */
> > +uint32_t rte_get_log_type(void);
> > +
> > +/**
> > * Get the current loglevel for the message being processed.
> > *
> > * Before calling the user-defined stream for logging, the log
> >
>
> Wouldn't it be better to change the variable:
> static struct log_cur_msg log_cur_msg[RTE_MAX_LCORE];
> into a pthread (tls) variable?
>
> With your patch, the log level and log type are not saved for
> non-EAL threads. If TLS were used, I think it would work in any case.
[LCM] Good point. But for this patch set, still suppose not involve big impact to EAL thread.
For improve non-EAL thread, we'll have a separate patch set for it.
>
> Regards,
> Olivier
Hi,
On 02/09/2015 03:19 PM, Liang, Cunming wrote:
>>> --- a/lib/librte_eal/common/include/rte_log.h
>>> +++ b/lib/librte_eal/common/include/rte_log.h
>>> @@ -144,6 +144,11 @@ uint32_t rte_get_log_level(void);
>>> void rte_set_log_type(uint32_t type, int enable);
>>>
>>> /**
>>> + * Get the global log type.
>>> + */
>>> +uint32_t rte_get_log_type(void);
>>> +
>>> +/**
>>> * Get the current loglevel for the message being processed.
>>> *
>>> * Before calling the user-defined stream for logging, the log
>>>
>>
>> Wouldn't it be better to change the variable:
>> static struct log_cur_msg log_cur_msg[RTE_MAX_LCORE];
>> into a pthread (tls) variable?
>>
>> With your patch, the log level and log type are not saved for
>> non-EAL threads. If TLS were used, I think it would work in any case.
> [LCM] Good point. But for this patch set, still suppose not involve big impact to EAL thread.
> For improve non-EAL thread, we'll have a separate patch set for it.
OK, that's fine
Will it be for 2.0 or later?
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Tuesday, February 10, 2015 1:45 AM
> To: Liang, Cunming; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 11/17] log: fix the gap to support non-EAL
> thread
>
> Hi,
>
> On 02/09/2015 03:19 PM, Liang, Cunming wrote:
> >>> --- a/lib/librte_eal/common/include/rte_log.h
> >>> +++ b/lib/librte_eal/common/include/rte_log.h
> >>> @@ -144,6 +144,11 @@ uint32_t rte_get_log_level(void);
> >>> void rte_set_log_type(uint32_t type, int enable);
> >>>
> >>> /**
> >>> + * Get the global log type.
> >>> + */
> >>> +uint32_t rte_get_log_type(void);
> >>> +
> >>> +/**
> >>> * Get the current loglevel for the message being processed.
> >>> *
> >>> * Before calling the user-defined stream for logging, the log
> >>>
> >>
> >> Wouldn't it be better to change the variable:
> >> static struct log_cur_msg log_cur_msg[RTE_MAX_LCORE];
> >> into a pthread (tls) variable?
> >>
> >> With your patch, the log level and log type are not saved for
> >> non-EAL threads. If TLS were used, I think it would work in any case.
> > [LCM] Good point. But for this patch set, still suppose not involve big impact to
> EAL thread.
> > For improve non-EAL thread, we'll have a separate patch set for it.
>
> OK, that's fine
>
> Will it be for 2.0 or later?
[LCM] Will be in 2.1 I suppose, together with the patch for mempool cache to support non-EAL thread.
@@ -193,11 +193,20 @@ rte_set_log_type(uint32_t type, int enable)
rte_logs.type &= (~type);
}
+/* Get global log type */
+uint32_t
+rte_get_log_type(void)
+{
+ return rte_logs.type;
+}
+
/* get the current loglevel for the message beeing processed */
int rte_log_cur_msg_loglevel(void)
{
unsigned lcore_id;
lcore_id = rte_lcore_id();
+ if (lcore_id >= RTE_MAX_LCORE)
+ return rte_get_log_level();
return log_cur_msg[lcore_id].loglevel;
}
@@ -206,6 +215,8 @@ int rte_log_cur_msg_logtype(void)
{
unsigned lcore_id;
lcore_id = rte_lcore_id();
+ if (lcore_id >= RTE_MAX_LCORE)
+ return rte_get_log_type();
return log_cur_msg[lcore_id].logtype;
}
@@ -265,8 +276,10 @@ rte_vlog(__attribute__((unused)) uint32_t level,
/* save loglevel and logtype in a global per-lcore variable */
lcore_id = rte_lcore_id();
- log_cur_msg[lcore_id].loglevel = level;
- log_cur_msg[lcore_id].logtype = logtype;
+ if (lcore_id < RTE_MAX_LCORE) {
+ log_cur_msg[lcore_id].loglevel = level;
+ log_cur_msg[lcore_id].logtype = logtype;
+ }
ret = vfprintf(f, format, ap);
fflush(f);
@@ -144,6 +144,11 @@ uint32_t rte_get_log_level(void);
void rte_set_log_type(uint32_t type, int enable);
/**
+ * Get the global log type.
+ */
+uint32_t rte_get_log_type(void);
+
+/**
* Get the current loglevel for the message being processed.
*
* Before calling the user-defined stream for logging, the log