Re: [PATCH v2] eal: avoid issues in macro expansion of alignment

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 4 Jul 2023 10:43:40 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Tuesday, 4 July 2023 01.24
> > 
> > RTE_ALIGN_MUL_NEAR is a macro so the cycle argument could
> > get evaluated twice causing some potential skew.  Fix by
> > computing value once.
> > 
> > Suggested by patch to fix side effects.
> > 
> > Fixes: 5cbd14b3e5f9 ("eal: roundup TSC frequency when estimating")
> > Cc: pbhagavatula@marvell.com
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > v2 - fix spelling error in commit message
> > 
> >  lib/eal/common/eal_common_timer.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/eal/common/eal_common_timer.c
> > b/lib/eal/common/eal_common_timer.c
> > index 5686a5102b66..05614b0503cf 100644
> > --- a/lib/eal/common/eal_common_timer.c
> > +++ b/lib/eal/common/eal_common_timer.c
> > @@ -42,10 +42,14 @@ estimate_tsc_freq(void)
> >  	RTE_LOG(WARNING, EAL, "WARNING: TSC frequency estimated roughly"
> >  		" - clock timings may be less accurate.\n");
> >  	/* assume that the rte_delay_us_sleep() will sleep for 1 second */
> > -	uint64_t start = rte_rdtsc();
> > +	uint64_t start, elapsed;
> > +
> > +	start = rte_rdtsc();
> >  	rte_delay_us_sleep(US_PER_S);
> > +	elapsed = rte_rdtsc() - start;
> > +
> >  	/* Round up to 10Mhz. 1E7 ~ 10Mhz */
> > -	return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, CYC_PER_10MHZ);
> > +	return RTE_ALIGN_MUL_NEAR(elapsed, CYC_PER_10MHZ);
> >  }
> > 
> >  void
> > --
> > 2.39.2  
> 
> Please fix the RTE_ALIGN_MUL_NEAR() macro instead. It already uses temporary variables with typeof() anyway.
> 
> Other macros might have similar behavior of using their parameters more than once, and could be improved too.
> 

It is already fixed, so this patch can be dropped.