All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brendan Jackman <jackmanb@google.com>
To: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	KP Singh <kpsingh@chromium.org>,
	Florent Revest <revest@chromium.org>,
	linux-kernel@vger.kernel.org, Jann Horn <jannh@google.com>
Subject: Re: [PATCH bpf-next v3 07/14] bpf: Add BPF_FETCH field / create atomic_fetch_add instruction
Date: Fri, 4 Dec 2020 09:12:21 +0000	[thread overview]
Message-ID: <X8n9dTz8oofFVWzM@google.com> (raw)
In-Reply-To: <45b3d69d-7356-2c28-e507-897889be564c@fb.com>

On Thu, Dec 03, 2020 at 09:27:04PM -0800, Yonghong Song wrote:
> On 12/3/20 8:02 AM, Brendan Jackman wrote:
[...]
> > diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c
> > index 37c8d6e9b4cc..3ee2246a52ef 100644
> > --- a/kernel/bpf/disasm.c
> > +++ b/kernel/bpf/disasm.c
> > @@ -160,6 +160,13 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
> >   				bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
> >   				insn->dst_reg, insn->off,
> >   				insn->src_reg);
> > +		} else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
> > +			   insn->imm == (BPF_ADD | BPF_FETCH)) {
> > +			verbose(cbs->private_data, "(%02x) r%d = atomic%s_fetch_add(*(%s *)(r%d %+d), r%d)\n",
> 
> We should not do dereference here (withough first *), right? since the input
> is actually an address. something like below?
>    r2 = atomic[64]_fetch_add((u64/u32 *)(r3 +40), r2)

Ah yep  - thanks!

[...]
> > diff --git a/tools/include/linux/filter.h b/tools/include/linux/filter.h
> > index 95ff51d97f25..ac7701678e1a 100644
> > --- a/tools/include/linux/filter.h
> > +++ b/tools/include/linux/filter.h
> > @@ -180,6 +180,16 @@
> >   		.imm   = BPF_ADD })
> >   #define BPF_STX_XADD BPF_ATOMIC_ADD /* alias */
> > +/* Atomic memory add with fetch, src_reg = atomic_fetch_add(*(dst_reg + off), src_reg); */
> 
> Maybe src_reg = atomic_fetch_add(dst_reg + off, src_reg)?

Yep - and the same for the bitwise ops in the later patch.


  reply	other threads:[~2020-12-04  9:13 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-03 16:02 [PATCH bpf-next v3 00/14] Atomics for eBPF Brendan Jackman
2020-12-03 16:02 ` [PATCH bpf-next v3 01/14] bpf: x86: Factor out emission of ModR/M for *(reg + off) Brendan Jackman
2020-12-03 16:02 ` [PATCH bpf-next v3 02/14] bpf: x86: Factor out emission of REX byte Brendan Jackman
2020-12-03 16:02 ` [PATCH bpf-next v3 03/14] bpf: x86: Factor out function to emit NEG Brendan Jackman
2020-12-03 16:02 ` [PATCH bpf-next v3 04/14] bpf: x86: Factor out a lookup table for some ALU opcodes Brendan Jackman
2020-12-03 16:02 ` [PATCH bpf-next v3 05/14] bpf: Rename BPF_XADD and prepare to encode other atomics in .imm Brendan Jackman
2020-12-04  4:49   ` Yonghong Song
2020-12-03 16:02 ` [PATCH bpf-next v3 06/14] bpf: Move BPF_STX reserved field check into BPF_STX verifier code Brendan Jackman
2020-12-04  4:51   ` Yonghong Song
2020-12-03 16:02 ` [PATCH bpf-next v3 07/14] bpf: Add BPF_FETCH field / create atomic_fetch_add instruction Brendan Jackman
2020-12-04  5:02   ` Yonghong Song
2020-12-04  5:27   ` Yonghong Song
2020-12-04  9:12     ` Brendan Jackman [this message]
2020-12-03 16:02 ` [PATCH bpf-next v3 08/14] bpf: Add instructions for atomic_[cmp]xchg Brendan Jackman
2020-12-04  5:34   ` Yonghong Song
2020-12-04  9:26     ` Brendan Jackman
2020-12-03 16:02 ` [PATCH bpf-next v3 09/14] bpf: Pull out a macro for interpreting atomic ALU operations Brendan Jackman
2020-12-04  6:30   ` Yonghong Song
2020-12-04  9:29     ` Brendan Jackman
2020-12-04 15:20       ` Yonghong Song
2020-12-03 16:02 ` [PATCH bpf-next v3 10/14] bpf: Add bitwise atomic instructions Brendan Jackman
2020-12-04  6:42   ` Yonghong Song
2020-12-04  9:36     ` Brendan Jackman
2020-12-04 15:21       ` Yonghong Song
2020-12-07 11:28         ` Brendan Jackman
2020-12-07 15:58           ` Yonghong Song
2020-12-07 16:14             ` Brendan Jackman
2020-12-03 16:02 ` [PATCH bpf-next v3 11/14] tools build: Implement feature check for BPF atomics in Clang Brendan Jackman
2020-12-03 21:02   ` Andrii Nakryiko
2020-12-03 16:02 ` [PATCH bpf-next v3 12/14] bpf: Pull tools/build/feature biz into selftests Makefile Brendan Jackman
2020-12-03 21:01   ` Andrii Nakryiko
2020-12-04  9:41     ` Brendan Jackman
2020-12-04 19:00       ` Andrii Nakryiko
2020-12-07 11:00         ` Brendan Jackman
2020-12-08  2:19           ` Andrii Nakryiko
2020-12-08 17:04             ` Brendan Jackman
2020-12-08 18:31               ` Andrii Nakryiko
2020-12-03 16:02 ` [PATCH bpf-next v3 13/14] bpf: Add tests for new BPF atomic operations Brendan Jackman
2020-12-04  7:06   ` Yonghong Song
2020-12-04  9:45     ` Brendan Jackman
2020-12-04 15:28       ` Yonghong Song
2020-12-04 19:49         ` Andrii Nakryiko
2020-12-07 15:48           ` Brendan Jackman
2020-12-03 16:02 ` [PATCH bpf-next v3 14/14] bpf: Document new atomic instructions Brendan Jackman
2020-12-03 16:10 ` [PATCH bpf-next v3 00/14] Atomics for eBPF Brendan Jackman
2020-12-04  4:46 ` 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=X8n9dTz8oofFVWzM@google.com \
    --to=jackmanb@google.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jannh@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.