> -----Original Message-----
> From: Kinsella, Ray <m...@ashroe.eu>
> Sent: Wednesday, July 8, 2020 11:05 PM
> To: David Marchand <david.march...@redhat.com>; Phil Yang
> <phil.y...@arm.com>; Aaron Conole <acon...@redhat.com>
> Cc: dev <dev@dpdk.org>; David Christensen <d...@linux.vnet.ibm.com>;
> Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; Ruifeng Wang
> <ruifeng.w...@arm.com>; nd <n...@arm.com>; Dodji Seketeli
> <do...@redhat.com>; Neil Horman <nhor...@tuxdriver.com>; Harman
> Kalra <hka...@marvell.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] eal: use c11 atomics for interrupt status
>
>
>
> On 08/07/2020 13:29, David Marchand wrote:
> > On Thu, Jun 11, 2020 at 12:25 PM Phil Yang <phil.y...@arm.com> wrote:
> >>
> >> The event status is defined as a volatile variable and shared
> >> between threads. Use c11 atomics with explicit ordering instead
> >> of rte_atomic ops which enforce unnecessary barriers on aarch64.
> >>
> >> Signed-off-by: Phil Yang <phil.y...@arm.com>
> >> Reviewed-by: Ruifeng Wang <ruifeng.w...@arm.com>
> >> ---
> >> lib/librte_eal/include/rte_eal_interrupts.h | 2 +-
> >> lib/librte_eal/linux/eal_interrupts.c | 47 ++++++++++++++++++++----
> -----
> >> 2 files changed, 34 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/lib/librte_eal/include/rte_eal_interrupts.h
> b/lib/librte_eal/include/rte_eal_interrupts.h
> >> index 773a34a..b1e8a29 100644
> >> --- a/lib/librte_eal/include/rte_eal_interrupts.h
> >> +++ b/lib/librte_eal/include/rte_eal_interrupts.h
> >> @@ -59,7 +59,7 @@ enum {
> >>
> >> /** interrupt epoll event obj, taken by epoll_event.ptr */
> >> struct rte_epoll_event {
> >> - volatile uint32_t status; /**< OUT: event status */
> >> + uint32_t status; /**< OUT: event status */
> >> int fd; /**< OUT: event fd */
> >> int epfd; /**< OUT: epoll instance the ev associated with */
> >> struct rte_epoll_data epdata;
> >
> > I got a reject from the ABI check in my env.
> >
> > 1 function with some indirect sub-type change:
> >
> > [C]'function int rte_pci_ioport_map(rte_pci_device*, int,
> > rte_pci_ioport*)' at pci.c:756:1 has some indirect sub-type changes:
> > parameter 1 of type 'rte_pci_device*' has sub-type changes:
> > in pointed to type 'struct rte_pci_device' at rte_bus_pci.h:57:1:
> > type size hasn't changed
> > 1 data member changes (2 filtered):
> > type of 'rte_intr_handle rte_pci_device::intr_handle' changed:
> > type size hasn't changed
> > 1 data member change:
> > type of 'rte_epoll_event rte_intr_handle::elist[512]' changed:
> > array element type 'struct rte_epoll_event' changed:
> > type size hasn't changed
> > 1 data member change:
> > type of 'volatile uint32_t rte_epoll_event::status'
> > changed:
> > entity changed from 'volatile uint32_t' to 'typedef
> > uint32_t' at stdint-uintn.h:26:1
> > type size hasn't changed
> >
> > type size hasn't changed
> >
> >
> > This is probably harmless in our case (going from volatile to non
> > volatile), but it won't pass the check in the CI without an exception
> > rule.
> >
> > Note: checking on the test-report ml, I saw nothing, but ovsrobot did
> > catch the issue with this change too, Aaron?
> >
> >
> Agreed, probably harmless and requires something in libagigail.ignore.
OK. Will update libagigail.ignore in the next version.
Thanks,
Phil
