From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Song Liu <liu.song.a23@gmail.com>
Cc: Song Liu <songliubraving@fb.com>,
Alexei Starovoitov <ast@kernel.org>,
"davem@davemloft.net" <davem@davemloft.net>,
"daniel@iogearbox.net" <daniel@iogearbox.net>,
"x86@kernel.org" <x86@kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH v2 bpf-next 03/17] bpf: Introduce BPF trampoline
Date: Thu, 7 Nov 2019 19:11:24 -0800 [thread overview]
Message-ID: <CAADnVQJFNo3wcyMKkOhX-LVYpgg302-K-As9ZKkPUXxRdGN0nw@mail.gmail.com> (raw)
In-Reply-To: <CAPhsuW4gYU=HJTe2ueDXhiyY__V1ZBF1ZEhCasHb5m8XgkTtww@mail.gmail.com>
On Thu, Nov 7, 2019 at 5:10 PM Song Liu <liu.song.a23@gmail.com> wrote:
> > > >>>>> + goto out;
> > > >>>>> + tr->selector++;
> > > >>>>
> > > >>>> Shall we do selector-- for unlink?
> > > >>>
> > > >>> It's a bit flip. I think it would be more confusing with --
> > > >>
> > > >> Right.. Maybe should use int instead of u64 for selector?
> > > >
> > > > No, since int can overflow.
> > >
> > > I guess it is OK to overflow, no?
> >
> > overflow is not ok, since transition 0->1 should use nop->call patching
> > whereas 1->2, 2->3 should use call->call.
> >
> > In my initial implementation (one I didn't share with anyone) I had
> > trampoline_mutex taken inside bpf_trampoline_update(). And multiple link()
> > operation were allowed. The idea was to attach multiple progs and update
> > trampoline once. But then I realized that I cannot do that since 'unlink +
> > update' where only 'update' is taking lock will not guarantee success. Since
> > other 'link' operations can race and 'update' can potentially fail in
> > arch_prepare_bpf_trampoline() due to new things that 'link' brought in. In that
> > version (since there several fentry/fexit progs can come in at once) I used
> > separate 'selector' ticker to pick the side of the page. Once I realized the
> > issue (to guarantee that unlink+update == always success) I moved mutex all the
> > way to unlink and link and left 'selector' as-is. Just now I realized that
> > 'selector' can be removed. fentry_cnt + fexit_cnt can be used instead. This
> > sum of counters will change 1 bit at a time. Am I right?
>
> Yeah, I think fentry_cnt + fexit_cnt is cleaner.
... and that didn't work.
It's transition that matters. Either need to remember previous sum value
or have separate selector. imo selector is cleaner, so I'm back to that.
next prev parent reply other threads:[~2019-11-08 3:11 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-07 5:46 [PATCH v2 bpf-next 00/17] Introduce BPF trampoline Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 01/17] bpf: refactor x86 JIT into helpers Alexei Starovoitov
2019-11-07 17:05 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 02/17] bpf: Add bpf_arch_text_poke() helper Alexei Starovoitov
2019-11-07 17:20 ` Song Liu
2019-11-07 17:50 ` Alexei Starovoitov
2019-11-07 17:54 ` Alexei Starovoitov
2019-11-07 18:04 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 03/17] bpf: Introduce BPF trampoline Alexei Starovoitov
2019-11-07 22:37 ` Song Liu
2019-11-07 22:55 ` Alexei Starovoitov
2019-11-07 23:07 ` Song Liu
2019-11-07 23:09 ` Alexei Starovoitov
2019-11-07 23:16 ` Song Liu
2019-11-08 0:09 ` Alexei Starovoitov
2019-11-08 1:10 ` Song Liu
2019-11-08 3:11 ` Alexei Starovoitov [this message]
2019-11-08 4:06 ` Song Liu
2019-11-08 4:08 ` Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 04/17] libbpf: Add support to attach to fentry/fexit tracing progs Alexei Starovoitov
2019-11-07 22:51 ` Song Liu
2019-11-07 23:07 ` Alexei Starovoitov
2019-11-07 23:13 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 05/17] selftest/bpf: Simple test for fentry/fexit Alexei Starovoitov
2019-11-07 23:22 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 06/17] bpf: Add kernel test functions for fentry testing Alexei Starovoitov
2019-11-07 23:28 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 07/17] selftests/bpf: Add test for BPF trampoline Alexei Starovoitov
2019-11-08 1:17 ` Song Liu
2019-11-08 2:33 ` Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 08/17] selftests/bpf: Add fexit tests " Alexei Starovoitov
2019-11-08 1:22 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 09/17] selftests/bpf: Add combined fentry/fexit test Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 10/17] selftests/bpf: Add stress test for maximum number of progs Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 11/17] bpf: Reserver space for BPF trampoline in BPF programs Alexei Starovoitov
2019-11-08 5:03 ` Andrii Nakryiko
2019-11-07 5:46 ` [PATCH v2 bpf-next 12/17] bpf: Fix race in btf_resolve_helper_id() Alexei Starovoitov
2019-11-08 5:13 ` Andrii Nakryiko
2019-11-08 5:31 ` Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 13/17] bpf: Compare BTF types of functions arguments with actual types Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 14/17] bpf: Support attaching tracing BPF program to other BPF programs Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 15/17] selftests/bpf: Extend test_pkt_access test Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 16/17] libbpf: Add support for attaching BPF programs to other BPF programs Alexei Starovoitov
2019-11-07 14:33 ` Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 17/17] selftests/bpf: Add a test for attaching BPF prog to another BPF prog and subprog Alexei Starovoitov
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=CAADnVQJFNo3wcyMKkOhX-LVYpgg302-K-As9ZKkPUXxRdGN0nw@mail.gmail.com \
--to=alexei.starovoitov@gmail.com \
--cc=Kernel-team@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=liu.song.a23@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=x86@kernel.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).