Re: [PATCH v2] twist: allow converting pr_devel()/pr_debug() into snprintf()

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Petr Mladek <pmladek@suse.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Ondrej Mosnacek <omosnace@redhat.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH v2] twist: allow converting pr_devel()/pr_debug() into snprintf()
Date: Thu, 28 May 2020 17:28:42 -0700	[thread overview]
Message-ID: <CAHk-=wjj9ooYACNvO2P_Gr_=aN0g=iEqtg0TwBJo18wbn4gthg@mail.gmail.com> (raw)
In-Reply-To: <13b0a475-e70e-c490-d34d-0c7a34facf7c@i-love.sakura.ne.jp>

On Thu, May 28, 2020 at 5:08 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> You mean "export these behavior as kernel command line options"? That will
> involve run-time costs (while build-time branching based on #ifdef can
> completely eliminate run-time costs).

Are _any_ of these things meaningful?

> Also, as number of options which
> can be controlled at boot-time grows, the kernel command line will become
> too long to specify all of these behavior.

So? We have explicitly added boot-config files for exactly this,
because people who do boot-time tracing wanted this.

> Why do you think "syzkaller is special" ? There is no syzkaller-specific
> choice.

ALL of these are designed to be totally about syzkaller. Nobody else
has ever asked for the TWIST options. And if they have, they'd still
make more sense as generic real actual options than as some odd
"twist" thing.

> Current kernel is not well segmented enough to allow switching based on
> per process flags. We can't distinguish whether some kernel message was
> caused by a process with such flags.

Who said anything at all about per process?

> All we could afford is to switch based on kernel boot command line. But
> that will entail a lot of code/data (and runtime-cost) which is not used
> if the administrator does not turn on the switches.

Absolutely nobody cares.

In fact, I'd prefer it just so that the options would be individually
explained, and not hidden away in some odd kernel config file, and
would be visible and force people to have sane nam,es.

> After all, switching at the kernel configuration phase is the most simple
> approach.

No it isn't. It's the UGLIEST possible approach, forcing a nasty
horrible config process to be even worse, forcing a compile-time
decision for something that isn't at all obvious should be
compile-time, and forcing crazy ugly config option names because they
are all just odd.

I have complained about this for MONTHS. You never never actually
explained why you want these badly named config options.

If it's something _so_ core that it affects performance, then no,
syzkaller shouldn't be messing with it in the first place, because
then you'd be testing something that is entirely irrelevant to anybody
else.

And if it's about things like "disable sysrq-k", and it might be
useful to somebody else than syzkaller, then it would be *much* better
off as a boot option so that you don't have to recompile the kernel to
pick it.

Some things might even be worth having a runtime option for.

But this "put random options, give them random names, and force them
all as compile-time options in a nasty kernel config process" just
smells.

And if they are _so_ specialized that only syzkaller could possibly
care, I still maintain that they shouldn't go upstream at all.

               Linus