Re: [dpdk-dev] [PATCH v2 1/3] eal/arm64: add 128-bit atomic compare exchange

[Jerin Jacob Kollanukkaran <jerinj@marvell.com>]

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Monday, June 24, 2019 11:11 PM
> To: thomas@monjalon.net; Phil Yang (Arm Technology China)
> <Phil.Yang@arm.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev@dpdk.org;
> hemant.agrawal@nxp.com; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; gage.eads@intel.com;
> nd <nd@arm.com>
> Subject: [EXT] RE: [PATCH v2 1/3] eal/arm64: add 128-bit atomic compare
> exchange
> >
> > 24/06/2019 18:12, Honnappa Nagarahalli:
> > > > > > +	} else {
> > > > > > +		rte_panic("Invalid memory order\n");
> > > > >
> > > > >
> > > > > rte_panic should be removed from library. In this case, I think,
> > > > > invalid mo can go for strongest barrier.
> > >
> > > It is added here to capture programming errors.
> > > Memory order can be passed during compilation or during run time.
> > > 'rte_panic' supports both of these.
> > > Adding code with strongest memory order will mask the programming error.
> >
> > An error must return a specific code from the function.
> > rte_panic is really forbidden in libraries.
> > We are in the process of removing all of them.
> Thank you for clarifying.
> In this particular use case, the API is similar to '__atomic_compare_exchange'
> built-in. Users would expect similar behavior. If we are differing from the
> standard behavior, we should document it in the API definition.

IMO, We should not differ from the standard behavior(return type) of atomic_compare_exchange.
And we should not have rte_panic in library. IMO, Best option would be
1) If mo is compile time constant then check with RTE_BUILD_ON for static assert to find invalid mo
2) if mo is runtime value and it is invalid then move to strongest memory order to make functionally
correct


> 
> >
> >