> > <snip previous discussions>
> >
> > > > diff --git a/lib/eal/common/rte_service.c
> > > > b/lib/eal/common/rte_service.c index ef31b1f63c..f045e74ef3 100644
> > > > --- a/lib/eal/common/rte_service.c
> > > > +++ b/lib/eal/common/rte_service.c
> > > > @@ -363,9 +363,15 @@ service_runner_do_callback(struct
> > > > rte_service_spec_impl *s,
> > > > uint64_t start = rte_rdtsc();
> > > > s->spec.callback(userdata);
> > > > uint64_t end = rte_rdtsc();
> > > > - s->cycles_spent += end - start;
> > > > + uint64_t cycles = end - start;
> > > > cs->calls_per_service[service_idx]++;
> > > > - s->calls++;
> > > > + if (service_mt_safe(s)) {
> > > > + __atomic_fetch_add(&s->cycles_spent, cycles,
> > > > __ATOMIC_RELAXED);
> > > > + __atomic_fetch_add(&s->calls, 1,
> > > > __ATOMIC_RELAXED);
> > > > + } else {
> > > > + s->cycles_spent += cycles;
> > > > + s->calls++;
> > > This is still a problem from a reader perspective. It is possible
> > > that the writes could be split while a reader is reading the stats. These
> need to be atomic adds.
> >
> > Thanks for pointing out; I do "think" in x86 in terms of load/store
> > tearing; and on x86 naturally aligned load/stores will not tear. Apologies
> > for
> missing the ARM angle here.
Arm architecture has similar things as well. I believe compiler does not
provide any guarantees that it will only generate non-tearing instructions.
Refer to a discussion in the past [1] [2] where it was thought that the
compiler would generate a non-tearing store (this is a slightly different
scenario).
[1] http://inbox.dpdk.org/dev/[email protected]/
[2] Refer to commit '316095eb41ed22da808b5826404c398917c83c89'
> >
> > I'm not sure how to best encode the difference between tearing & "locked
> instructions"
> > to make things multi-writer safe. But they're not the same thing, and
> > I'd prefer not pay the penalty for LOCK instructions (multi-writer) only to
> satisfy the non-tearing requirements.
> >
> > Is there an rte_atomic-* type that is guaranteed as non-tearing?
Nothing that I know of.
> >
> > In that case, changing the type of the calls/cycles_spent variables to such
> > a
> type to ensure "non-tearing"
> > single-reader, single-writer behaviour is enough, instead of forcing
> __atomic_fetch_add() everywhere?
>
> Regular read, increment and then atomic store should work without locks
> where alignment is correct on most architectures, and doing the store
> atomically should prevent any tearing.
Agree, we could do this, will provide more flexibility for the
micro-architecture to work with. Good to understand the perf benefits vs
complexity and the branch cost.
