On Wed, 3 Mar 2021 at 08:14, Björn Töpel <bjorn.topel@xxxxxxxxx> wrote:
>
> On 2021-03-03 05:38, Andrii Nakryiko wrote:
> > On Mon, Mar 1, 2021 at 2:43 AM Björn Töpel <bjorn.topel@xxxxxxxxx> wrote:
> >>
> >> From: Björn Töpel <bjorn.topel@xxxxxxxxx>
> >>
> >> Now that the AF_XDP rings have load-acquire/store-release semantics,
> >> move libbpf to that as well.
> >>
> >> The library-internal libbpf_smp_{load_acquire,store_release} are only
> >> valid for 32-bit words on ARM64.
> >>
> >> Also, remove the barriers that are no longer in use.
> >>
> >> Signed-off-by: Björn Töpel <bjorn.topel@xxxxxxxxx>
> >> ---
> >> tools/lib/bpf/libbpf_util.h | 72 +++++++++++++++++++++++++------------
> >> tools/lib/bpf/xsk.h | 17 +++------
> >> 2 files changed, 55 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/libbpf_util.h b/tools/lib/bpf/libbpf_util.h
> >> index 59c779c5790c..94a0d7bb6f3c 100644
> >> --- a/tools/lib/bpf/libbpf_util.h
> >> +++ b/tools/lib/bpf/libbpf_util.h
> >> @@ -5,6 +5,7 @@
> >> #define __LIBBPF_LIBBPF_UTIL_H
> >>
> >> #include <stdbool.h>
> >> +#include <linux/compiler.h>
> >>
> >> #ifdef __cplusplus
> >> extern "C" {
> >> @@ -15,29 +16,56 @@ extern "C" {
> >> * application that uses libbpf.
> >> */
> >> #if defined(__i386__) || defined(__x86_64__)
> >> -# define libbpf_smp_rmb() asm volatile("" : : : "memory")
> >> -# define libbpf_smp_wmb() asm volatile("" : : : "memory")
> >> -# define libbpf_smp_mb() \
> >> - asm volatile("lock; addl $0,-4(%%rsp)" : : : "memory", "cc")
> >> -/* Hinders stores to be observed before older loads. */
> >> -# define libbpf_smp_rwmb() asm volatile("" : : : "memory")
> >
> > So, technically, these four are part of libbpf's API, as libbpf_util.h
> > is actually installed on target hosts. Seems like xsk.h is the only
> > one that is using them, though.
> >
> > So the question is whether it's ok to remove them now?
> >
>
> I would say that. Ideally, the barriers shouldn't be visible at all,
> since they're only used as an implementation detail for the static
> inline functions.
>
>
> > And also, why wasn't this part of xsk.h in the first place?
> >
>
> I guess there was a "maybe it can be useful for more than the XDP socket
> parts of libbpf"-idea. I'll move them to xsk.h for the v2, which will
> make the migration easier.
>
Clarification! The reason for not having them in xsk.h, was that the
idea was that only the APIs allowed from the application should reside
there. IOW, libbpf_utils.h is only "implementation details". Again,
the static-inline function messes things up. Maybe moving to an
LTO-only world would be better, so we can get rid of the inlining all
together.
>
> Björn
>
>
> >> +# define libbpf_smp_store_release(p, v) \
> >> + do { \
> >> + asm volatile("" : : : "memory"); \
> >> + WRITE_ONCE(*p, v); \
> >> + } while (0)
> >> +# define libbpf_smp_load_acquire(p) \
> >> + ({ \
> >> + typeof(*p) ___p1 = READ_ONCE(*p); \
> >> + asm volatile("" : : : "memory"); \
> >> + ___p1; \
> >> + })
> >
> > [...]
> >
