Re: [dpdk-dev] [PATCH 2/3] eventdev: use c11 atomics for lcore timer armed flag

[Phil Yang <Phil.Yang@arm.com>]

> -----Original Message-----
> From: Carrillo, Erik G <erik.g.carrillo@intel.com>
> Sent: Wednesday, June 24, 2020 5:32 AM
> To: Stephen Hemminger <stephen@networkplumber.org>; Phil Yang
> <Phil.Yang@arm.com>
> Cc: dev@dpdk.org; drc@linux.vnet.ibm.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> nd <nd@arm.com>
> Subject: RE: [dpdk-dev] [PATCH 2/3] eventdev: use c11 atomics for lcore
> timer armed flag
> 
> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Tuesday, June 23, 2020 4:20 PM
> > To: Phil Yang <phil.yang@arm.com>
> > Cc: dev@dpdk.org; Carrillo, Erik G <erik.g.carrillo@intel.com>;
> > drc@linux.vnet.ibm.com; honnappa.nagarahalli@arm.com;
> > ruifeng.wang@arm.com; dharmik.thakkar@arm.com; nd@arm.com
> > Subject: Re: [dpdk-dev] [PATCH 2/3] eventdev: use c11 atomics for lcore
> > timer armed flag
> >
> > On Fri, 12 Jun 2020 19:19:57 +0800
> > Phil Yang <phil.yang@arm.com> wrote:
> >
> > >  	/* Track which cores have actually armed a timer */
> > >  	struct {
> > > -		rte_atomic16_t v;
> > > +		int16_t v;
> > >  	} __rte_cache_aligned in_use[RTE_MAX_LCORE];
> >
> > Do you really need this to be cache aligned (ie one per line)?
> 
> I believe I did this originally to keep a cache line from bouncing when two
> different cores are arming timers, so it's not strictly necessary.

Yeah, if we remove it, these per core variables might cause a false sharing issue between threads. 
That will hurt performance.

> 
> > Why have a signed value for a reference count? Shouldn't it be unsigned?

Yes. It should be unsigned in the new code. 
I will update it in the next version.

Thanks,
Phil