Re: [dpdk-dev] [PATCH 2/2] eal: use c11 atomics for interrupt status

[David Marchand <david.marchand@redhat.com>]

On Thu, Jun 11, 2020 at 12:25 PM Phil Yang <phil.yang@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.yang@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@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?


-- 
David Marchand