[dpdk-dev] [PATCH v4 11/17] log: fix the gap to support non-EAL thread
> -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Tuesday, February 10, 2015 1:45 AM > To: Liang, Cunming; dev at 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.
[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?
[dpdk-dev] [PATCH v4 11/17] log: fix the gap to support non-EAL thread
> -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Monday, February 09, 2015 4:01 AM > To: Liang, Cunming; dev at 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 > > --- > > 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
[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 > --- > 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
[dpdk-dev] [PATCH v4 11/17] log: fix the gap to support non-EAL thread
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 --- 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 -- 1.8.1.4