From: "Mattias Rönnblom" <hofors@lysator.liu.se>
To: "Morten Brørup" <mb@smartsharesystems.com>,
"Mattias Rönnblom" <mattias.ronnblom@ericsson.com>,
dev@dpdk.org, "Tyler Retzlaff" <roretzla@linux.microsoft.com>
Cc: Heng Wang <heng.wang@ericsson.com>,
Stephen Hemminger <stephen@networkplumber.org>,
techboard@dpdk.org
Subject: Re: [RFC v2 5/6] eal: add atomic bit operations
Date: Sun, 28 Apr 2024 17:37:41 +0200 [thread overview]
Message-ID: <93464b2a-f454-4b4f-ba10-94701ee4ef72@lysator.liu.se> (raw)
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F3F6@smartserver.smartshare.dk>
On 2024-04-26 14:00, Morten Brørup wrote:
>> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se]
>> Sent: Friday, 26 April 2024 11.39
>>
>> On 2024-04-25 18:18, Morten Brørup wrote:
>>>> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se]
>>>> Sent: Thursday, 25 April 2024 16.36
>>>>
>>>> On 2024-04-25 12:25, Morten Brørup wrote:
>>>>>> +#define rte_bit_atomic_test(addr, nr, memory_order)
>>>> \
>>>>>> + _Generic((addr), \
>>>>>> + uint32_t *: __rte_bit_atomic_test32, \
>>>>>> + uint64_t *: __rte_bit_atomic_test64)(addr, nr,
>>>> memory_order)
>>>>>
>>>>> I wonder if these should have RTE_ATOMIC qualifier:
>>>>>
>>>>> + RTE_ATOMIC(uint32_t) *: __rte_bit_atomic_test32,
>>>> \
>>>>> + RTE_ATOMIC(uint64_t) *: __rte_bit_atomic_test64)(addr, nr,
>>>> memory_order)
>>>>>
>>>>>
>>>>>> +#define __RTE_GEN_BIT_ATOMIC_TEST(size)
>>>> \
>>>>>> + static inline bool \
>>>>>> + __rte_bit_atomic_test ## size(const uint ## size ## _t *addr,
>>>> \
>>>>>
>>>>> I wonder if the "addr" parameter should have RTE_ATOMIC qualifier:
>>>>>
>>>>> + __rte_bit_atomic_test ## size(const RTE_ATOMIC(uint ## size ## _t)
>>>> *addr, \
>>>>>
>>>>> instead of casting into a_addr.
>>>>>
>>>>
>>>> Check the cover letter for the rationale for the cast.
>>>
>>> Thanks, that clarifies it. Then...
>>> For the series:
>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>
>>>>
>>>> Where I'm at now is that I think C11 _Atomic is rather poor design. The
>>>> assumption that an object which allows for atomic access always should
>>>> require all operations upon it to be atomic, regardless of where it is
>>>> in its lifetime, and which thread is accessing it, does not hold, in the
>>>> general case.
>>>
>>> It might be slow, but I suppose the C11 standard prioritizes correctness
>> over performance.
>>>
>>
>> That's a false dichotomy, in this case. You can have both.
>
> In theory you shouldn't need non-atomic access to atomic variables.
> In reality, we want it anyway, because real CPUs are faster at non-atomic operations.
>
>>
>>> It seems locks are automatically added to _Atomic types larger than what is
>> natively supported by the architecture.
>>> E.g. MSVC adds locks to _Atomic types larger than 8 byte. [1]
>>>
>>> [1]: https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-
>> 2022-version-17-5-preview-2/
>>>
>>>>
>>>> The only reason for _Atomic being as it is, as far as I can see, is to
>>>> accommodate for ISAs which does not have the appropriate atomic machine
>>>> instructions, and thus require a lock or some other data associated with
>>>> the actual user-data-carrying bits. Neither GCC nor DPDK supports any
>>>> such ISAs, to my knowledge. I suspect neither never will. So the cast
>>>> will continue to work.
>>>
>>> I tend to agree with you on this.
>>>
>>> We should officially decide that DPDK treats RTE_ATOMIC types as a union of
>> _Atomic and non-atomic, i.e. operations on RTE_ATOMIC types can be both atomic
>> and non-atomic.
>>>
>>
>> I think this is a subject which needs to be further explored.
>
> Yes. It's easier exploring and deciding now, when our options are open, than after we have locked down the affected APIs.
>
>>
>> Objects that can be accessed both atomically and non-atomically should
>> be without _Atomic. With my current understanding of this issue, that
>> seems like the best option.
>
> Agree.
>
> The alterative described below is certainly no good!
>
> It would be nice if they were marked as sometimes-atomic by a qualifier or special type, like rte_be32_t marks the network byte order variant of an uint32_t.
>
> Furthermore, large atomic objects need the _Atomic qualifier for the compiler to add (and use) the associated lock.
If you have larger objects than the ISA can handle, you wouldn't want to
leave the choice of the synchronization primitive to use to the
compiler. I don't see how it could possibly know, which one is the most
appropriate, especially in a DPDK context. It would for example need to
know if the contending threads are non-preemptable or not.
In some situations a sequence lock may well be your best option. Will
the compiler generate one for you?
If "lock" means std::mutex, it would be a disaster, performance-wise.
> Alternatively, we could specify that sometimes-atomic objects cannot be larger than 8 byte, which is what MSVC can handle without locking.
>
>>
>> You could turn it around as well, and have such marked _Atomic and have
>> explicit casts to their non-_Atomic cousins when operated upon by
>> non-atomic functions. Not sure how realistic that is, since
>> non-atomicity is the norm. All generic selection-based "functions" must
>> take this into account.
>>
>>>>
>>>>>> + unsigned int nr, int memory_order) \
>>>>>> + { \
>>>>>> + RTE_ASSERT(nr < size); \
>>>>>> + \
>>>>>> + const RTE_ATOMIC(uint ## size ## _t) *a_addr = \
>>>>>> + (const RTE_ATOMIC(uint ## size ## _t) *)addr; \
>>>>>> + uint ## size ## _t mask = (uint ## size ## _t)1 << nr; \
>>>>>> + return rte_atomic_load_explicit(a_addr, memory_order) &
>>>> mask; \
>>>>>> + }
>>>>>
>>>>>
>>>>> Similar considerations regarding volatile qualifier for the "once"
>>>> operations.
>>>>>
next prev parent reply other threads:[~2024-04-28 15:37 UTC|newest]
Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-02 13:53 [RFC 0/7] Improve EAL bit operations API Mattias Rönnblom
2024-03-02 13:53 ` [RFC 1/7] eal: extend bit manipulation functions Mattias Rönnblom
2024-03-02 17:05 ` Stephen Hemminger
2024-03-03 6:26 ` Mattias Rönnblom
2024-03-04 16:34 ` Tyler Retzlaff
2024-03-05 18:01 ` Mattias Rönnblom
2024-03-05 18:06 ` Tyler Retzlaff
2024-04-25 8:58 ` [RFC v2 0/6] Improve EAL bit operations API Mattias Rönnblom
2024-04-25 8:58 ` [RFC v2 1/6] eal: extend bit manipulation functionality Mattias Rönnblom
2024-04-29 9:51 ` [RFC v3 0/6] Improve EAL bit operations API Mattias Rönnblom
2024-04-29 9:51 ` [RFC v3 1/6] eal: extend bit manipulation functionality Mattias Rönnblom
2024-04-29 11:12 ` Morten Brørup
2024-04-30 9:55 ` [RFC v4 0/6] Improve EAL bit operations API Mattias Rönnblom
2024-04-30 9:55 ` [RFC v4 1/6] eal: extend bit manipulation functionality Mattias Rönnblom
2024-04-30 12:08 ` [RFC v5 0/6] Improve EAL bit operations API Mattias Rönnblom
2024-04-30 12:08 ` [RFC v5 1/6] eal: extend bit manipulation functionality Mattias Rönnblom
2024-05-02 5:57 ` [RFC v6 0/6] Improve EAL bit operations API Mattias Rönnblom
2024-05-02 5:57 ` [RFC v6 1/6] eal: extend bit manipulation functionality Mattias Rönnblom
2024-05-05 8:37 ` [RFC v7 0/6] Improve EAL bit operations API Mattias Rönnblom
2024-05-05 8:37 ` [RFC v7 1/6] eal: extend bit manipulation functionality Mattias Rönnblom
2024-05-05 8:37 ` [RFC v7 2/6] eal: add unit tests for bit operations Mattias Rönnblom
2024-05-05 8:37 ` [RFC v7 3/6] eal: add exactly-once bit access functions Mattias Rönnblom
2024-05-07 19:17 ` Morten Brørup
2024-05-08 6:47 ` Mattias Rönnblom
2024-05-08 7:33 ` Morten Brørup
2024-05-08 8:00 ` Mattias Rönnblom
2024-05-08 8:11 ` Morten Brørup
2024-05-08 9:27 ` Mattias Rönnblom
2024-05-08 10:08 ` Morten Brørup
2024-05-08 15:15 ` Stephen Hemminger
2024-05-08 16:16 ` Morten Brørup
2024-05-05 8:37 ` [RFC v7 4/6] eal: add unit tests for " Mattias Rönnblom
2024-05-05 8:37 ` [RFC v7 5/6] eal: add atomic bit operations Mattias Rönnblom
2024-05-05 8:37 ` [RFC v7 6/6] eal: add unit tests for atomic bit access functions Mattias Rönnblom
2024-05-02 5:57 ` [RFC v6 2/6] eal: add unit tests for bit operations Mattias Rönnblom
2024-05-02 5:57 ` [RFC v6 3/6] eal: add exactly-once bit access functions Mattias Rönnblom
2024-05-02 5:57 ` [RFC v6 4/6] eal: add unit tests for " Mattias Rönnblom
2024-05-02 5:57 ` [RFC v6 5/6] eal: add atomic bit operations Mattias Rönnblom
2024-05-03 6:41 ` Mattias Rönnblom
2024-05-03 23:30 ` Tyler Retzlaff
2024-05-04 15:36 ` Mattias Rönnblom
2024-05-02 5:57 ` [RFC v6 6/6] eal: add unit tests for atomic bit access functions Mattias Rönnblom
2024-04-30 12:08 ` [RFC v5 2/6] eal: add unit tests for bit operations Mattias Rönnblom
2024-04-30 12:08 ` [RFC v5 3/6] eal: add exactly-once bit access functions Mattias Rönnblom
2024-04-30 12:08 ` [RFC v5 4/6] eal: add unit tests for " Mattias Rönnblom
2024-04-30 12:08 ` [RFC v5 5/6] eal: add atomic bit operations Mattias Rönnblom
2024-04-30 12:08 ` [RFC v5 6/6] eal: add unit tests for atomic bit access functions Mattias Rönnblom
2024-04-30 9:55 ` [RFC v4 2/6] eal: add unit tests for bit operations Mattias Rönnblom
2024-04-30 9:55 ` [RFC v4 3/6] eal: add exactly-once bit access functions Mattias Rönnblom
2024-04-30 9:55 ` [RFC v4 4/6] eal: add unit tests for " Mattias Rönnblom
2024-04-30 10:37 ` Morten Brørup
2024-04-30 11:58 ` Mattias Rönnblom
2024-04-30 9:55 ` [RFC v4 5/6] eal: add atomic bit operations Mattias Rönnblom
2024-04-30 9:55 ` [RFC v4 6/6] eal: add unit tests for atomic bit access functions Mattias Rönnblom
2024-04-29 9:51 ` [RFC v3 2/6] eal: add unit tests for bit operations Mattias Rönnblom
2024-04-29 9:51 ` [RFC v3 3/6] eal: add exactly-once bit access functions Mattias Rönnblom
2024-04-29 9:51 ` [RFC v3 4/6] eal: add unit tests for " Mattias Rönnblom
2024-04-29 9:51 ` [RFC v3 5/6] eal: add atomic bit operations Mattias Rönnblom
2024-04-29 9:51 ` [RFC v3 6/6] eal: add unit tests for atomic bit access functions Mattias Rönnblom
2024-04-25 8:58 ` [RFC v2 2/6] eal: add unit tests for bit operations Mattias Rönnblom
2024-04-25 8:58 ` [RFC v2 3/6] eal: add exactly-once bit access functions Mattias Rönnblom
2024-04-25 8:58 ` [RFC v2 4/6] eal: add unit tests for " Mattias Rönnblom
2024-04-25 8:58 ` [RFC v2 5/6] eal: add atomic bit operations Mattias Rönnblom
2024-04-25 10:25 ` Morten Brørup
2024-04-25 14:36 ` Mattias Rönnblom
2024-04-25 16:18 ` Morten Brørup
2024-04-26 9:39 ` Mattias Rönnblom
2024-04-26 12:00 ` Morten Brørup
2024-04-28 15:37 ` Mattias Rönnblom [this message]
2024-04-29 7:24 ` Morten Brørup
2024-04-30 16:52 ` Tyler Retzlaff
2024-04-25 8:58 ` [RFC v2 6/6] eal: add unit tests for atomic bit access functions Mattias Rönnblom
2024-04-25 18:05 ` [RFC v2 0/6] Improve EAL bit operations API Tyler Retzlaff
2024-04-26 11:17 ` Mattias Rönnblom
2024-04-26 21:35 ` Patrick Robb
2024-03-02 13:53 ` [RFC 2/7] eal: add generic bit manipulation macros Mattias Rönnblom
2024-03-04 8:16 ` Heng Wang
2024-03-04 15:41 ` Mattias Rönnblom
2024-03-04 16:42 ` Tyler Retzlaff
2024-03-05 18:08 ` Mattias Rönnblom
2024-03-05 18:22 ` Tyler Retzlaff
2024-03-05 20:02 ` Mattias Rönnblom
2024-03-05 20:53 ` Tyler Retzlaff
2024-03-02 13:53 ` [RFC 3/7] eal: add bit manipulation functions which read or write once Mattias Rönnblom
2024-03-02 13:53 ` [RFC 4/7] eal: add generic once-type bit operations macros Mattias Rönnblom
2024-03-02 13:53 ` [RFC 5/7] eal: add atomic bit operations Mattias Rönnblom
2024-03-02 13:53 ` [RFC 6/7] eal: add generic " Mattias Rönnblom
2024-03-02 13:53 ` [RFC 7/7] eal: deprecate relaxed family of " Mattias Rönnblom
2024-03-02 17:07 ` Stephen Hemminger
2024-03-03 6:30 ` Mattias Rönnblom
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=93464b2a-f454-4b4f-ba10-94701ee4ef72@lysator.liu.se \
--to=hofors@lysator.liu.se \
--cc=dev@dpdk.org \
--cc=heng.wang@ericsson.com \
--cc=mattias.ronnblom@ericsson.com \
--cc=mb@smartsharesystems.com \
--cc=roretzla@linux.microsoft.com \
--cc=stephen@networkplumber.org \
--cc=techboard@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).