Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1605560917.git.dxu@dxuuu.xyz> <470ffc3c76414443fc359b884080a5394dcccec3.1605560917.git.dxu@dxuuu.xyz>
 <CAHk-=wggUw3XYffJ-od8Dbfh-JkXkEuCPjSRR2Z+8HrNUNxJ=g@mail.gmail.com>
In-Reply-To: <CAHk-=wggUw3XYffJ-od8Dbfh-JkXkEuCPjSRR2Z+8HrNUNxJ=g@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon, 16 Nov 2020 14:44:56 -0800
Message-ID: <CAHk-=wiEgTXYgLXg8YxRHnH+eZno800pEp8caskKgDCgq55s+g@mail.gmail.com>
Subject: Re: [PATCH bpf v6 1/2] lib/strncpy_from_user.c: Don't overcopy bytes
 after NUL terminator
To:     Daniel Xu <dxu@dxuuu.xyz>, Matt Turner <mattst88@gmail.com>,
        Richard Henderson <rth@twiddle.net>,
        Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc:     bpf <bpf@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Song Liu <songliubraving@fb.com>, andrii.nakryiko@gmail.com,
        kernel-team@fb.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Nov 16, 2020 at 2:15 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So I've verified that at least on x86-64, this doesn't really make
> code generation any worse, and I'm ok with the patch from that
> standpoint.

.. looking closer, it will generate extra code on big-endian
architectures and on alpha, because of the added "zero_bytemask()".
But on the usual LE machines, zero_bytemask() will already be the same
as "mask", so all it adds is that "and" operation with values it
already had access to.

I don't think anybody cares about alpha and BE - traditional BE
architectures have moved to LE anyway. And looking at the alpha
word-at-a-time code, I don't even understand how it works at all.

Adding matt/rth/ivan to the cc, just so that maybe one of them can
educate me on how that odd alpha zero_bytemask() could possibly work.
The "2ul << .." part confuses me, I think it should be "1ul << ...".

I get the feeling that the alpha "2ul" constant might have come from
the tile version, but in the tile version, the " __builtin_ctzl()"
counts the leading zeroes to the top bit of any bytes in 'mask'. But
the alpha version actually uses "find_zero(mask) * 8", so rather than
have values of 7/15/23/... (for zero byte in byte 0/1/2/..
respectively), it has values 0/8/16/....

But it's entirely possible that I'm completely confused, and alpha
does it right, and I'm just not understanding the code.

It's also possible that the "2ul" vs "1ul" case doesn't matter.
because the extra bit is always going to mask the byte that is
actually zero, so being one bit off in the result is a non-event. I
think that is what may actually be going on.

                Linus