linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: John Fastabend <john.fastabend@gmail.com>
Cc: Yonghong Song <yhs@fb.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Brendan Jackman <jackmanb@google.com>, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	KP Singh <kpsingh@chromium.org>,
	Florent Revest <revest@chromium.org>,
	open list <linux-kernel@vger.kernel.org>,
	Jann Horn <jannh@google.com>
Subject: Re: [PATCH v2 bpf-next 00/13] Atomics for eBPF
Date: Tue, 1 Dec 2020 21:59:03 -0800	[thread overview]
Message-ID: <CAEf4BzY0vHvrj4jAc+qszSMcYABd0dGFVxkM-d8S=wwHoDFD=A@mail.gmail.com> (raw)
In-Reply-To: <5fc72beede900_15eb720850@john-XPS-13-9370.notmuch>

On Tue, Dec 1, 2020 at 9:53 PM John Fastabend <john.fastabend@gmail.com> wrote:
>
> Yonghong Song wrote:
> >
> >
>
> [...]
>
> > > Great, this means that all existing valid uses of
> > > __sync_fetch_and_add() will generate BPF_XADD instructions and will
> > > work on old kernels, right?
> >
> > That is correct.
> >
> > >
> > > If that's the case, do we still need cpu=v4? The new instructions are
> > > *only* going to be generated if the user uses previously unsupported
> > > __sync_fetch_xxx() intrinsics. So, in effect, the user consciously
> > > opts into using new BPF instructions. cpu=v4 seems like an unnecessary
> > > tautology then?
> >
> > This is a very good question. Essentially this boils to when users can
> > use the new functionality including meaningful return value  of
> > __sync_fetch_and_add().
> >    (1). user can write a small bpf program to test the feature. If user
> >         gets a failed compilation (fatal error), it won't be supported.
> >         Otherwise, it is supported.
> >    (2). compiler provides some way to tell user it is safe to use, e.g.,
> >         -mcpu=v4, or some clang macro suggested by Brendan earlier.
> >
> > I guess since kernel already did a lot of feature discovery. Option (1)
> > is probably fine.
>
> For option (2) we can use BTF with kernel version check. If kernel is
> greater than kernel this lands in we use the the new instructions if
> not we use a slower locked version. That should work for all cases
> unless someone backports patches into an older case.

Two different things: Clang support detection and kernel support
detection. You are talking about kernel detection, I and Yonghong were
talking about Clang detection and explicit cpu=v4 opt-in.

For kernel detection, if there is an enum value or type that gets
added along the feature, then with CO-RE built-ins it's easy to detect
and kernel dead code elimination will make sure that unsupported
instructions won't trip up the BPF verifier. Still need Clang support
to compile the program in the first place, though.

If there is no such BTF-based way to check, it is still possible to
try to load a trivial BPF program with __sync_fech_and_xxx() to do
feature detection and then use .rodata to turn off code paths relying
on a new instruction set.

>
> At least thats what I'll probably end up wrapping in a helper function.

  reply	other threads:[~2020-12-02  6:00 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-27 17:57 [PATCH v2 bpf-next 00/13] Atomics for eBPF Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 01/13] bpf: x86: Factor out emission of ModR/M for *(reg + off) Brendan Jackman
2020-11-29  1:15   ` Alexei Starovoitov
2020-12-01 12:14     ` Brendan Jackman
2020-12-02  5:50       ` Alexei Starovoitov
2020-12-02 10:52         ` Brendan Jackman
2020-12-02 17:35           ` Alexei Starovoitov
2020-11-27 17:57 ` [PATCH v2 bpf-next 02/13] bpf: x86: Factor out emission of REX byte Brendan Jackman
2020-11-29  1:14   ` Alexei Starovoitov
2020-12-01 12:12     ` Brendan Jackman
2020-12-02  5:48       ` Alexei Starovoitov
2020-12-02 10:54         ` Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 03/13] bpf: x86: Factor out function to emit NEG Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 04/13] bpf: x86: Factor out a lookup table for some ALU opcodes Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 05/13] bpf: Rename BPF_XADD and prepare to encode other atomics in .imm Brendan Jackman
2020-11-28  3:43   ` Yonghong Song
2020-12-01 12:17     ` Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 06/13] bpf: Move BPF_STX reserved field check into BPF_STX verifier code Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 07/13] bpf: Add BPF_FETCH field / create atomic_fetch_add instruction Brendan Jackman
2020-11-28  4:15   ` Yonghong Song
2020-12-01 12:22     ` Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 08/13] bpf: Add instructions for atomic_[cmp]xchg Brendan Jackman
2020-11-28  5:25   ` Yonghong Song
2020-12-01 12:27     ` Brendan Jackman
2020-11-29  1:27   ` Alexei Starovoitov
2020-12-01 12:32     ` Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 09/13] bpf: Pull out a macro for interpreting atomic ALU operations Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 10/13] bpf: Add instructions for atomic[64]_[fetch_]sub Brendan Jackman
2020-11-27 21:39   ` kernel test robot
2020-11-27 21:39   ` [RFC PATCH] bpf: bpf_atomic_alu_string[] can be static kernel test robot
2020-11-28  5:35   ` [PATCH v2 bpf-next 10/13] bpf: Add instructions for atomic[64]_[fetch_]sub Yonghong Song
2020-11-29  1:34     ` Alexei Starovoitov
2020-11-30 17:18       ` Yonghong Song
2020-12-01 12:38         ` Brendan Jackman
2020-12-02  5:55           ` Alexei Starovoitov
2020-12-02 11:19             ` Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 11/13] bpf: Add bitwise atomic instructions Brendan Jackman
2020-11-28  5:39   ` Yonghong Song
2020-11-29  1:36     ` Alexei Starovoitov
2020-11-30 17:20       ` Yonghong Song
2020-11-27 17:57 ` [PATCH v2 bpf-next 12/13] bpf: Add tests for new BPF atomic operations Brendan Jackman
2020-12-01  3:55   ` Yonghong Song
2020-12-01 12:56     ` Brendan Jackman
2020-12-01 17:24       ` Yonghong Song
2020-12-02  2:22   ` Andrii Nakryiko
2020-12-02 12:26     ` Brendan Jackman
2020-11-27 17:57 ` [PATCH v2 bpf-next 13/13] bpf: Document new atomic instructions Brendan Jackman
2020-11-28  5:53 ` [PATCH v2 bpf-next 00/13] Atomics for eBPF Yonghong Song
2020-11-29  1:40   ` Alexei Starovoitov
2020-11-30 17:22     ` Yonghong Song
2020-12-01  3:48       ` Yonghong Song
2020-12-02  2:00         ` Andrii Nakryiko
2020-12-02  5:05           ` Yonghong Song
2020-12-02  5:53             ` John Fastabend
2020-12-02  5:59               ` Andrii Nakryiko [this message]
2020-12-02  6:27                 ` John Fastabend
2020-12-02  8:03             ` Yonghong Song

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='CAEf4BzY0vHvrj4jAc+qszSMcYABd0dGFVxkM-d8S=wwHoDFD=A@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jackmanb@google.com \
    --cc=jannh@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kpsingh@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=revest@chromium.org \
    --cc=yhs@fb.com \
    /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).