MIME-Version: 1.0
In-Reply-To: <CAHk-=wh-eTh=4g28Ec5W4pHNTaCSZWJdxVj4BH2sNE2hAA+cww@mail.gmail.com>
References: <CAHk-=wgbm1rjkSs0w+dVJJzzK2M1No=j419c+i7T4V4ky2skOw@mail.gmail.com>
 <20230302083025.khqdizrnjkzs2lt6@wittgenstein> <CAHk-=wivxuLSE4ESRYv_=e8wXrD0GEjFQmUYnHKyR1iTDTeDwg@mail.gmail.com>
 <CAGudoHF9WKoKhKRHOH_yMsPnX+8Lh0fXe+y-K26mVR0gajEhaQ@mail.gmail.com>
 <ZADoeOiJs6BRLUSd@ZenIV> <CAGudoHFhnJ1z-81FKYpzfDmvcWFeHNkKGdr00CkuH5WJa2FAMQ@mail.gmail.com>
 <CAHk-=wjp5fMupRwnROtC5Yn+MVLA7v=J+_QJSi1rr3qAjdsfXw@mail.gmail.com>
 <CAHk-=wi11ZbOBdMR5hQDz0x0NNZ9gM-4SxXxK-7R3_yh7e10rQ@mail.gmail.com>
 <ZAD21ZEiB2V9Ttto@ZenIV> <6400fedb.170a0220.ece29.04b8@mx.google.com>
 <ZAEC3LN6oUe6BKSN@ZenIV> <CAG_fn=UQEuvJ9WXou_sW3moHcVQZJ9NvJ5McNcsYE8xw_WEYGw@mail.gmail.com>
 <CAGudoHFqNdXDJM2uCQ9m7LzP0pAx=iVj1WBnKc4k9Ky1Xf5XmQ@mail.gmail.com> <CAHk-=wh-eTh=4g28Ec5W4pHNTaCSZWJdxVj4BH2sNE2hAA+cww@mail.gmail.com>
From:   Mateusz Guzik <mjguzik@gmail.com>
Date:   Fri, 3 Mar 2023 20:37:02 +0100
Message-ID: <CAGudoHG+anGcO1XePmLjb+Hatr4VQMiZ2FufXs8hT3JrHyGMAw@mail.gmail.com>
Subject: Re: [PATCH v3 2/2] vfs: avoid duplicating creds in faccessat if possible
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Alexander Potapenko <glider@google.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Kees Cook <keescook@chromium.org>,
        Eric Biggers <ebiggers@google.com>,
        Christian Brauner <brauner@kernel.org>, serge@hallyn.com,
        paul@paul-moore.com, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On 3/3/23, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Fri, Mar 3, 2023 at 9:39=E2=80=AFAM Mateusz Guzik <mjguzik@gmail.com> =
wrote:
>>
>> So the key is: memset is underperforming at least on x86-64 for
>> certain sizes and the init-on-alloc thingy makes it used significantly
>> more, exacerbating the problem
>
> One reason that the kernel memset isn't as optimized as memcpy, is
> simply because under normal circumstances it shouldn't be *used* that
> much outside of page clearing and constant-sized structure
> initialization.
>

I mentioned in the previous e-mail that memset is used a lot even
without the problematic opt and even have shown size distribution of
what's getting passed there.

You made me curious how usage compares to memcpy, so I checked 'make'
in kernel dir once again.

I stress the init-on-alloc opt is *OFF*:
# CONFIG_INIT_ON_ALLOC_DEFAULT_ON is not set
# CONFIG_INIT_ON_FREE_DEFAULT_ON is not set

# bpftrace -e 'kprobe:memset,kprobe:memcpy { @[probe] =3D count(); }'
[snip]
@[kprobe:memcpy]: 6948498
@[kprobe:memset]: 36150671

iow memset is used about 7 times as much as memcpy when building the
kernel. If it matters I'm building on top of
2eb29d59ddf02e39774abfb60b2030b0b7e27c1f (reasonably fresh master).

> So this is literally a problem with pointless automated memset,
> introduced by that hardening option. And hardening people generally
> simply don't care about performance, and the people who _do _care
> about performance usually don't enable the known-expensive crazy
> stuff.
>
> Honestly, I think the INIT_ONCE stuff is actively detrimental, and
> only hides issues (and in this case adds its own). So I can't but help
> to say "don't do that then". I think it's literally stupid to clear
> allocations by default.
>

Questioning usability of the entire thing was on the menu in what I
intended to write, but I did not read entire public discussion so
perhaps I missed a crucial insight.

> I'm not opposed to improving memset, but honestly, if the argument is
> based on the stupid hardening behavior, I really think *that* needs to
> be fixed first.
>

It is not. The argument is that this is a core primitive, used a lot
with sizes where rep stos is detrimental to its performance. There is
no urgency, but eventually someone(tm) should sort it out. For
$reasons I don't want to do it myself.

I do bring it up in the context of the init-on-alloc machinery because
it disfigures whatever results are obtained testing on x86-64 -- they
can get exactly the same effect for much smaller cost, consequently
they should have interest in sorting this out.

General point though was that the work should have sanity-checked
performance of the primitive it relies on, instead of assuming it is
~optimal. I'm guilty of this mistake myself, so not going to throw
stones.

--=20
Mateusz Guzik <mjguzik gmail.com>