From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Puranjay Mohan <puranjay12@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
"Russell King \(Oracle\)" <linux@armlinux.org.uk>,
"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
Song Liu <song@kernel.org>, Stanislav Fomichev <sdf@google.com>,
Yonghong Song <yonghong.song@linux.dev>,
Shubham Bansal <illusionist.neo@gmail.com>,
linux-riscv <linux-riscv@lists.infradead.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Helge Deller <deller@gmx.de>,
John Fastabend <john.fastabend@gmail.com>,
Andrii Nakryiko <andrii@kernel.org>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
Xi Wang <xi.wang@gmail.com>, Albert Ou <aou@eecs.berkeley.edu>,
Luke Nelson <luke.r.nels@gmail.com>,
Nicholas Piggin <npiggin@gmail.com>,
KP Singh <kpsingh@kernel.org>,
Paul Walmsley <paul.walmsley@sifive.com>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
Hao Luo <haoluo@google.com>,
linux-parisc@vger.kernel.org,
ppc-dev <linuxppc-dev@lists.ozlabs.org>,
Wang YanQing <udknight@gmail.com>,
LKML <linux-kernel@vger.kernel.org>,
Palmer Da bbelt <palmer@dabbelt.com>,
Jiri Olsa <jolsa@kernel.org>,
Network Development <netdev@vger.kernel.org>,
bpf <bpf@vger.kernel.org>,
Martin KaFai Lau <martin.lau@linux.dev>
Subject: Re: [PATCH bpf-next 5/6] bpf, arm32: Always zero extend for LDX with B/H/W
Date: Tue, 12 Sep 2023 17:10:42 -0700 [thread overview]
Message-ID: <CAADnVQLzbyG3xWVDFyTsDPRSC=fnAskaeyc1erQVLYo_b6Lg_w@mail.gmail.com> (raw)
In-Reply-To: <CANk7y0iFdgHgu+RXYJvP3swaRS+-Lr0CgOAdcQWtjs4VkrOzdQ@mail.gmail.com>
On Tue, Sep 12, 2023 at 4:17 PM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> On Wed, Sep 13, 2023 at 1:04 AM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Sep 12, 2023 at 10:46:53PM +0000, Puranjay Mohan wrote:
> > > The JITs should not depend on the verifier for zero extending the upper
> > > 32 bits of the destination register when loading a byte, half-word, or
> > > word.
> > >
> > > A following patch will make the verifier stop patching zext instructions
> > > after LDX.
> >
> > This was introduced by:
> >
> > 163541e6ba34 ("arm: bpf: eliminate zero extension code-gen")
> >
> > along with an additional function. So three points:
> >
> > 1) the commit should probably explain why it has now become undesirable
> > to access this verifier state, whereas it appears it was explicitly
> > added to permit this optimisation.
>
> I added some details in the cover letter.
>
> For the complete discussion see: [1]
>
> > 2) you state that jits should not depend on this state, but the above
> > commit adds more references than you're removing, so aren't there still
> > references to the verifier remaining after this patch? I count a total
> > of 10, and the patch below removes three.
>
> The JITs should not depend on this state for LDX (loading
> a B/H/W.
> This patch removes the usage only for LDX.
>
> > 3) what about the bpf_jit_needs_zext() function that was added to
> > support the export of this zext state?
>
> That is still applicable, The verifier will still emit zext
> instructions for other
> instructions like BPF_ALU / BPF_ALU64
>
> >
> > Essentially, the logic stated in the commit message doesn't seem to be
> > reflected by the proposed code change.
>
> I will try to provide more information.
> Currently I have asked Alexei if we really need this in [2].
> I still think this optimization is useful and we should keep it.
Right. subreg tracking is indeed functional for narrow loads.
Let's drop this patch set.
next prev parent reply other threads:[~2023-09-13 0:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-12 22:46 [PATCH bpf-next 0/6] bpf: verifier: stop emitting zext for LDX Puranjay Mohan
2023-09-12 22:46 ` [PATCH bpf-next 1/6] bpf, riscv32: Always zero extend for LDX with B/W/H Puranjay Mohan
2023-09-12 22:46 ` [PATCH bpf-next 2/6] bpf, x86-32: " Puranjay Mohan
2023-09-12 22:46 ` [PATCH bpf-next 3/6] bpf, parisc32: " Puranjay Mohan
2023-09-12 22:46 ` [PATCH bpf-next 4/6] bpf, powerpc32: Always zero extend for LDX Puranjay Mohan
2023-09-12 22:46 ` [PATCH bpf-next 5/6] bpf, arm32: Always zero extend for LDX with B/H/W Puranjay Mohan
2023-09-12 23:03 ` Russell King (Oracle)
2023-09-12 23:16 ` Puranjay Mohan
2023-09-13 0:10 ` Alexei Starovoitov [this message]
2023-09-12 22:46 ` [PATCH bpf-next 6/6] bpf, verifier: always mark destination of LDX as 64-bit Puranjay Mohan
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='CAADnVQLzbyG3xWVDFyTsDPRSC=fnAskaeyc1erQVLYo_b6Lg_w@mail.gmail.com' \
--to=alexei.starovoitov@gmail.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=andrii@kernel.org \
--cc=aou@eecs.berkeley.edu \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=deller@gmx.de \
--cc=haoluo@google.com \
--cc=illusionist.neo@gmail.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux@armlinux.org.uk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=luke.r.nels@gmail.com \
--cc=martin.lau@linux.dev \
--cc=naveen.n.rao@linux.ibm.com \
--cc=netdev@vger.kernel.org \
--cc=npiggin@gmail.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=puranjay12@gmail.com \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=udknight@gmail.com \
--cc=xi.wang@gmail.com \
--cc=yonghong.song@linux.dev \
/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).