From: Vlastimil Babka <vbabka@suse.cz>
To: Luis Chamberlain <mcgrof@kernel.org>, Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
linux-mm@kvack.org, Ivan Teterevkov <ivan.teterevkov@nutanix.com>,
Michal Hocko <mhocko@kernel.org>,
David Rientjes <rientjes@google.com>,
Matthew Wilcox <willy@infradead.org>,
"Eric W . Biederman" <ebiederm@xmission.com>,
"Guilherme G . Piccoli" <gpiccoli@canonical.com>,
Alexey Dobriyan <adobriyan@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Christian Brauner <christian.brauner@ubuntu.com>,
Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [PATCH 1/3] kernel/sysctl: support setting sysctl parameters from kernel command line
Date: Tue, 14 Apr 2020 13:25:07 +0200 [thread overview]
Message-ID: <7585f0b0-c5d2-b527-aac7-eeafdd15ffad@suse.cz> (raw)
In-Reply-To: <20200406170822.GE11244@42.do-not-panic.com>
On 4/6/20 7:08 PM, Luis Chamberlain wrote:
> On Mon, Apr 06, 2020 at 08:58:50AM -0700, Kees Cook wrote:
>> On Mon, Apr 06, 2020 at 02:08:36PM +0000, Luis Chamberlain wrote:
>> > > Yes. Doing an internal extension isn't testing the actual code.
>> >
>> > But it would.
>> >
>> > [...]
>> > > I don't think anything is needed for this series. It can be boot tested
>> > > manually.
>> >
>> > Why test it manually when it could be tested automatically with a new kconfig?
>>
>> So, my impression is that adding code to the internals to test the
>> internals isn't a valid test (or at least makes it fragile) because the
>> test would depend on the changes to the internals (or at least depend on
>> non-default non-production CONFIGs).
>
> The *internal* aspect here is an extension to boot params under a
> kconfig which would simply append to it, as if the user would have
So there's no such kconfig yet to apply boot parameters specified by configure,
right? That would itself be a new feature. Or could we use bootconfig? (CC Masami)
> added some more params. Since we already have test sysctl params the
> only one we'd need to add on the test driver would be a dummy one which
> tests the alias, on the second patch. We should have enough sysctls to
> already test dummy values.
Right.
> Nothing else would be needed as the sysctl test driver would just need
> to test that the values expected when this is enabled is set.
>
>> Can you send a patch for what you think this should look like? Perhaps
>> I'm not correctly imagining what you're describing?
>
> I rather get the person involved in the changes to do the testing so
> as they're the ones designing the feature. If however it is not clear
> what I mean I'm happy to elaborate.
>
> Vlastimil do you get what I mean?
Hopefully :)
>> Regardless of testing, I think this series is ready for -mm.
>
> I'm happy for it to go in provided we at least devise a follow up plan
> for testing. Otherwise -- like other things, it won't get done.
OK I'll send a v2 and we can discuss the test driver details. I don't want to
neglect testing, but also not block the new functionality, in case it means
testing means adding another new functionality.
Thanks,
Vlastimil
> Luis
>
next prev parent reply other threads:[~2020-04-14 11:25 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-30 11:55 [PATCH 0/3] support setting sysctl parameters from kernel command line Vlastimil Babka
2020-03-30 11:55 ` [PATCH 1/3] kernel/sysctl: " Vlastimil Babka
2020-03-30 16:23 ` Vlastimil Babka
2020-03-30 17:40 ` Kees Cook
2020-03-30 17:39 ` Kees Cook
2020-03-30 22:44 ` Luis Chamberlain
2020-03-31 7:42 ` Vlastimil Babka
2020-03-31 7:48 ` Michal Hocko
2020-03-31 18:26 ` Kees Cook
2020-03-31 14:31 ` Luis Chamberlain
2020-04-01 11:01 ` Vlastimil Babka
2020-04-02 16:04 ` Luis Chamberlain
2020-04-02 17:23 ` Kees Cook
2020-04-02 20:59 ` Luis Chamberlain
2020-04-03 23:57 ` Kees Cook
2020-04-06 14:08 ` Luis Chamberlain
2020-04-06 15:58 ` Kees Cook
2020-04-06 17:08 ` Luis Chamberlain
2020-04-14 11:25 ` Vlastimil Babka [this message]
2020-04-15 3:23 ` Masami Hiramatsu
2020-04-15 6:08 ` Luis Chamberlain
2020-03-30 11:55 ` [PATCH 2/3] kernel/sysctl: support handling command line aliases Vlastimil Babka
2020-03-30 17:41 ` Kees Cook
2020-03-31 14:35 ` Luis Chamberlain
2020-03-30 11:55 ` [PATCH 3/3] kernel/hung_task convert hung_task_panic boot parameter to sysctl Vlastimil Babka
2020-03-30 17:43 ` Kees Cook
2020-03-31 0:34 ` John Hubbard
2020-03-31 7:27 ` Vlastimil Babka
2020-03-31 15:49 ` John Hubbard
2020-03-31 23:12 ` Tetsuo Handa
2020-04-01 8:47 ` Vlastimil Babka
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=7585f0b0-c5d2-b527-aac7-eeafdd15ffad@suse.cz \
--to=vbabka@suse.cz \
--cc=adobriyan@gmail.com \
--cc=christian.brauner@ubuntu.com \
--cc=ebiederm@xmission.com \
--cc=gpiccoli@canonical.com \
--cc=gregkh@linuxfoundation.org \
--cc=ivan.teterevkov@nutanix.com \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mcgrof@kernel.org \
--cc=mhiramat@kernel.org \
--cc=mhocko@kernel.org \
--cc=rientjes@google.com \
--cc=tglx@linutronix.de \
--cc=willy@infradead.org \
--cc=yzaikin@google.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 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).