> -----Original Message-----
> From: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
> Sent: Thursday, July 7, 2022 6:26 PM
> To: Van Haaren, Harry <harry.van.haa...@intel.com>; Morten Brørup
> <m...@smartsharesystems.com>; Mattias Rönnblom <hof...@lysator.liu.se>;
> mattias.ronnblom <mattias.ronnb...@ericsson.com>; dev@dpdk.org
> Cc: nd <n...@arm.com>; nd <n...@arm.com>
> Subject: RE: Service core statistics MT safety
>
> <snip>
<snip>
> > Yes, understood and agree. Per-core counters are preferred, as relaxed
> > atomic
> > ordered stores are enough to ensure non-tearing loads. Summation before
> > reporting back from the stats-requesting thread.
> > For backporting, per-core counters is a significant change. Is using
> > atomics to fix
> > the miss-behaviour a better idea?
> Agree, the change will be simple.
>
> This will result in some perf impact which can be mitigated by disabling the
> stats
> where possible.
Yes, agree, disabling stats is a workaround if they're not required yep.
Correctness > performance. We can optimize to per-CPU stats in a future release.
> > My question below is still open, is the below enough to fix the
> > *functional* part
> > of MT services?
> >
> > Code today uses normal ADD/INCQ (and hence the MT increments bug of
> > tear/clobber exists)
> > 0x000055555649189d <+141>: add %rax,0x50(%rbx)
> > 0x00005555564918a1 <+145>: incq 0x48(%rbx)
> >
> > For x86, my disassembly for RELAXED and SEQ_CST orderings are the same,
> > using LOCK prefix ("proper atomics")
> > 0x000055555649189d <+141>: lock add %rax,0x50(%rbx)
> > 0x00005555564918a2 <+146>: lock incq 0x48(%rbx)
> >
> > My understanding is that since 2x threads can write the same address,
> > SEQ_CST
> > is required.
> > Can somebody more familiar with C11 confirm that?
> We need to use RELAXED. You can think of it as, 'do we need a particular
> order for
> memory operations within a single thread?' In this case, we do not need a
> particular
> order for incrementing stats in the single thread context.
Thanks for confirming; makes sense.
2 patches sent, you're on CC.
