All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Vernet <void@manifault.com>
To: Joanne Koong <joannelkoong@gmail.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	andrii@kernel.org, john.fastabend@gmail.com,
	martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
	kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
	jolsa@kernel.org, tj@kernel.org, linux-kernel@vger.kernel.org,
	Kernel-team@fb.com
Subject: Re: [PATCH 1/5] bpf: Clear callee saved regs after updating REG0
Date: Tue, 9 Aug 2022 05:47:24 -0700	[thread overview]
Message-ID: <20220809124724.ps6fmzeazizzjoon@dev0025.ash9.facebook.com> (raw)
In-Reply-To: <CAJnrk1bxiYfaR-2aM-PQdg75UQxWt0XJZxxrMs3sfZo02vvkYw@mail.gmail.com>

On Mon, Aug 08, 2022 at 04:32:39PM -0700, Joanne Koong wrote:

[...]

> > It being a read-only const is was why I made this a BUILD_BUG_ON. My
> > intention here was to ensure that we're not accidentally skipping the
> > resetting of caller_saved[0]. The original code iterated from
> > caller_saved[0] -> caller_saved[CALLER_SAVED_REGS - 1]. Now that we're
> > starting from caller_saved[1], this compile-time assertion verifies that
> > we're not accidentally skipping caller_saved[0] by checking that it's the
> > same as BPF_REG_0, which is reset above. Does that make sense?
> 
> I think it's an invariant that r0 - r5 are the caller saved args and
> that caller_saved[0] will always be BPF_REG_0. I'm having a hard time
> seeing a case where this would change in the future, but then again, I
> am also not a fortune teller so maybe I am wrong here :) I don't think
> it's a big deal though so I don't feel strongly about this

I agree that it seems very unlikely to change, but I don't see the harm in
leaving it in. Compile time checks are very fast, and are meant for cases
such as these to check constant, build-time invariants. If you feel
strongly, I can remove it.

Thanks,
David

      reply	other threads:[~2022-08-09 12:47 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-08 15:53 [PATCH 1/5] bpf: Clear callee saved regs after updating REG0 David Vernet
2022-08-08 15:53 ` [PATCH 2/5] bpf: Define new BPF_MAP_TYPE_USER_RINGBUF map type David Vernet
2022-08-11 23:22   ` Andrii Nakryiko
2022-08-12 16:21     ` David Vernet
2022-08-11 23:29   ` Andrii Nakryiko
2022-08-12 16:23     ` David Vernet
2022-08-16 18:43       ` Andrii Nakryiko
2022-08-08 15:53 ` [PATCH 3/5] bpf: Add bpf_user_ringbuf_drain() helper David Vernet
2022-08-08 21:55   ` kernel test robot
2022-08-11 23:22   ` Andrii Nakryiko
2022-08-12  0:01     ` David Vernet
2022-08-12  0:46     ` David Vernet
2022-08-16 18:57       ` Andrii Nakryiko
2022-08-16 22:14         ` David Vernet
2022-08-16 22:59           ` Andrii Nakryiko
2022-08-17 20:24             ` David Vernet
2022-08-17 21:03               ` David Vernet
2022-08-08 15:53 ` [PATCH 4/5] bpf: Add libbpf logic for user-space ring buffer David Vernet
2022-08-11 23:39   ` Andrii Nakryiko
2022-08-12 17:28     ` David Vernet
2022-08-16 19:09       ` Andrii Nakryiko
2022-08-17 14:02         ` David Vernet
2022-08-18  3:05           ` David Vernet
2022-08-08 15:53 ` [PATCH 5/5] selftests/bpf: Add selftests validating the user ringbuf David Vernet
2022-08-08 18:14 ` [PATCH 1/5] bpf: Clear callee saved regs after updating REG0 Joanne Koong
2022-08-08 18:50   ` David Vernet
2022-08-08 23:32     ` Joanne Koong
2022-08-09 12:47       ` David Vernet [this message]

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=20220809124724.ps6fmzeazizzjoon@dev0025.ash9.facebook.com \
    --to=void@manifault.com \
    --cc=Kernel-team@fb.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=joannelkoong@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=tj@kernel.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.