Re: + mm-shuffle-default-enable-all-shuffling.patch added to -mm tree

Re: + mm-shuffle-default-enable-all-shuffling.patch added to -mm tree

[Michal Hocko]

On Wed 06-02-19 12:02:54, Andrew Morton wrote:
> From: Dan Williams <dan.j.williams@intel.com>
> Subject: mm/shuffle: default enable all shuffling
> 
> Per Andrew's request arrange for all memory allocation shuffling code to
> be enabled by default.
> 
> The page_alloc.shuffle command line parameter can still be used to disable
> shuffling at boot, but the kernel will default enable the shuffling if the
> command line option is not specified.
> 
> Link: http://lkml.kernel.org/r/154943713572.3858443.11206307988382889377.stgit@dwillia2-desk3.amr.corp.intel.com
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Keith Busch <keith.busch@intel.com>
> 
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

I hope this is mmotm only thing and even then, is this really
something we want for linux-next? There are people doing testing and
potentially performance testing on that tree. Do we want to invalidate
all that work? I can see some argument about a testing coverage but do
we really need it for the change like this? The randomization is quite
simple to review and I assume Dan has given this good testing before
submition.
-- 
Michal Hocko
SUSE Labs

Re: + mm-shuffle-default-enable-all-shuffling.patch added to -mm tree

[Andrew Morton]

On Tue, 12 Feb 2019 09:54:28 +0100 Michal Hocko <mhocko@kernel.org> wrote:

> On Wed 06-02-19 12:02:54, Andrew Morton wrote:
> > From: Dan Williams <dan.j.williams@intel.com>
> > Subject: mm/shuffle: default enable all shuffling
> > 
> > Per Andrew's request arrange for all memory allocation shuffling code to
> > be enabled by default.
> > 
> > The page_alloc.shuffle command line parameter can still be used to disable
> > shuffling at boot, but the kernel will default enable the shuffling if the
> > command line option is not specified.
> > 
> > Link: http://lkml.kernel.org/r/154943713572.3858443.11206307988382889377.stgit@dwillia2-desk3.amr.corp.intel.com
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Michal Hocko <mhocko@suse.com>
> > Cc: Dave Hansen <dave.hansen@linux.intel.com>
> > Cc: Keith Busch <keith.busch@intel.com>
> > 
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> I hope this is mmotm only thing and even then, is this really
> something we want for linux-next? There are people doing testing and
> potentially performance testing on that tree. Do we want to invalidate
> all that work? I can see some argument about a testing coverage but do
> we really need it for the change like this? The randomization is quite
> simple to review and I assume Dan has given this good testing before
> submition.

Please see the mailing list discussion.  Without this patch the feature
is likely to end up in mainline with next to no testing other than Dan's.

If it disrupts people's performance testing then whoops, sorry, but we
wanted to know about that!

Re: + mm-shuffle-default-enable-all-shuffling.patch added to -mm tree

[Michal Hocko]

On Tue 12-02-19 13:46:22, Andrew Morton wrote:
> On Tue, 12 Feb 2019 09:54:28 +0100 Michal Hocko <mhocko@kernel.org> wrote:
> 
> > On Wed 06-02-19 12:02:54, Andrew Morton wrote:
> > > From: Dan Williams <dan.j.williams@intel.com>
> > > Subject: mm/shuffle: default enable all shuffling
> > > 
> > > Per Andrew's request arrange for all memory allocation shuffling code to
> > > be enabled by default.
> > > 
> > > The page_alloc.shuffle command line parameter can still be used to disable
> > > shuffling at boot, but the kernel will default enable the shuffling if the
> > > command line option is not specified.
> > > 
> > > Link: http://lkml.kernel.org/r/154943713572.3858443.11206307988382889377.stgit@dwillia2-desk3.amr.corp.intel.com
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > Cc: Kees Cook <keescook@chromium.org>
> > > Cc: Michal Hocko <mhocko@suse.com>
> > > Cc: Dave Hansen <dave.hansen@linux.intel.com>
> > > Cc: Keith Busch <keith.busch@intel.com>
> > > 
> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > 
> > I hope this is mmotm only thing and even then, is this really
> > something we want for linux-next? There are people doing testing and
> > potentially performance testing on that tree. Do we want to invalidate
> > all that work? I can see some argument about a testing coverage but do
> > we really need it for the change like this? The randomization is quite
> > simple to review and I assume Dan has given this good testing before
> > submition.
> 
> Please see the mailing list discussion.  Without this patch the feature
> is likely to end up in mainline with next to no testing other than Dan's.

Isn't that the case for most of the functionality behind CONFIG_$FOO
that doesn't get enabled by default?

It is not that I care too much but I find this way of argumentation
strange. It is the submitter to make sure the feature is tested properly
and reviewers should make sure the overall design and implementation
makes sense. I do not see reason why all users of linux-next should be a
guinea pigs without knowing that.
-- 
Michal Hocko
SUSE Labs