From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Song Liu <songliubraving@fb.com>
Cc: Song Liu <liu.song.a23@gmail.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 20:08:13 -0800 [thread overview]
Message-ID: <CAADnVQLz9fWBm3qYWaw9n60WOHFWtzO1RXheHYj6nd+jg--TkA@mail.gmail.com> (raw)
In-Reply-To: <3000B3E1-25DE-4653-B11C-AAF61492B0FF@fb.com>
On Thu, Nov 7, 2019 at 8:06 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Nov 7, 2019, at 7:11 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >
> > 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.
>
> Hmm.. is this because of the error handling path?
No. Because of transition 1->2 and 2->1 are the same.
next prev parent reply other threads:[~2019-11-08 4:08 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
2019-11-08 4:06 ` Song Liu
2019-11-08 4:08 ` Alexei Starovoitov [this message]
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=CAADnVQLz9fWBm3qYWaw9n60WOHFWtzO1RXheHYj6nd+jg--TkA@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).