Re: [dpdk-dev] [PATCH] timer: add lfence before TSC read

["François-Frédéric Ozog" <ff@ozog.com>]

Hi,

Most of the time rdtsc is used for timestamping and a few cycles incorrect
are most of the time not an issue (a precision of 0.1us for session start is
usually enough).

Sometimes you need to serialize because the time you want to measure is very
short, in the order of few nanoseconds.

If the code is running in a VM, which usually virtualize rdtsc instruction,
then it even make no sense to have more "precision".

IMHO, adding the lfence for all cases is introducing an un-necessary
performance penalty.

What about adding rte_rdtsc_sync() or rte_rdtsc_serial() with the comment
about the rdtsc instruction behavior so that developers can choose which
form they want?

François-Frédéric


> -----Message d'origine-----
> De : dev [mailto:dev-bounces@dpdk.org] De la part de Didier Pallard
> Envoyé : vendredi 24 janvier 2014 12:18
> À : dev@dpdk.org
> Objet : [dpdk-dev] [PATCH] timer: add lfence before TSC read
> 
> According to Intel Developer's Manual:
> 
> "The RDTSC instruction is not a serializing instruction. It does not
> necessarily wait  until all previous instructions have been executed
before
> reading the counter. Simi-  larly, subsequent instructions may begin
> execution before the read operation is  performed. If software requires
> RDTSC to be executed only after all previous instruc-  tions have
completed
> locally, it can either use RDTSCP (if the processor supports that
>  instruction) or execute the sequence LFENCE;RDTSC."
> 
> So add a lfence instruction before rdtsc to synchronize read operations
and
> ensure that the read is done at the expected instant.
> 
> Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> ---
>  lib/librte_eal/common/include/rte_cycles.h |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/librte_eal/common/include/rte_cycles.h
> b/lib/librte_eal/common/include/rte_cycles.h
> index cc6fe71..487dba6 100644
> --- a/lib/librte_eal/common/include/rte_cycles.h
> +++ b/lib/librte_eal/common/include/rte_cycles.h
> @@ -110,6 +110,9 @@ rte_rdtsc(void)
>  		};
>  	} tsc;
> 
> +	/* serialize previous load instructions in pipe */
> +	asm volatile("lfence");
> +
>  #ifdef RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT
>  	if (unlikely(rte_cycles_vmware_tsc_map)) {
>  		/* ecx = 0x10000 corresponds to the physical TSC for VMware
*/
> --
> 1.7.10.4