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

["Kinsella, Ray" <mdr@ashroe.eu>]

On 08/07/2020 13:29, David Marchand wrote:
> 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?
> 
> 
Agreed, probably harmless and requires something in libagigail.ignore.