Hi, Totally agree with Fran?ois-Fr?d?ric. Actually was going to suggest exactly the same thing. BTW, there is rte_rmb() defined inside rte_atomic.h, that should produce an lfence instruction. Probably better to use it to keep code consistent. Konstantin
-----Original Message----- From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Fran?ois-Fr?d?ric Ozog Sent: Friday, January 24, 2014 11:43 AM To: 'Didier Pallard'; dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH] timer: add lfence before TSC read 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 at dpdk.org] De la part de Didier Pallard > Envoy??: vendredi 24 janvier 2014 12:18 ??: dev at 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 at 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 -------------------------------------------------------------- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
