Re: [dpdk-dev] [PATCH v4 11/17] log: fix the gap to support non-EAL thread

["Liang, Cunming" <cunming.liang@intel.com>]

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