Re: [GIT PULL] MM updates for 5.20-rc1

[Joe Perches <joe@perches.com>]

On Fri, 2022-08-05 at 17:07 -0700, Andrew Morton wrote:
> On Fri, 5 Aug 2022 17:04:28 -0700 Roman Gushchin <roman.gushchin@linux.dev> wrote:
> 
> > On Fri, Aug 05, 2022 at 04:32:34PM -0700, Linus Torvalds wrote:
> > > On Wed, Aug 3, 2022 at 1:35 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > 
> > > > - memcg debug/visibility work from Roman Gushchin
> > > 
> > > Well, not just memcg.
> > > 
> > > There's that new CONFIG_SHRINKER_DEBUG too.
> > > 
> > > Which looks like yet another "people add Kconfig options incorrectly".
> > > 
> > > We don't make new features "default y" unless there's some truly
> > > pressing reason for it (ie "99.9% of all people will want this" or "we
> > > used to always compile this in, now it's optional").
> > 
> > Hi Linus!
> > 
> > It actually was 'default n' in one of the earlier version of the patchset
> > and has been switched to 'default y' based on the following feedback from
> > Dave Chinner (https://lore.kernel.org/lkml/YmiWK56bOHyrr64u@rh/):
> > 
> >     No. The argument that "if we turn it off there's no overhead" means
> >     one of two things:
> > 
> >     1. nobody turns it on and it never gets tested and so bitrots and is
> >     useless, or
> >     2. distro's all turn it on because some tool they ship or customer
> >     they ship to wants it.
> > 
> >     Either way, hiding it behind a config option is not an acceptible
> >     solution for mering poorly thought out infrastructure.
> > 
> > Personally I think that the feature is not that useful for the majority
> > of users (this is why default was n), but it's not adding much of the
> > overhead, so I had no strong reasons to oppose Dave.
> > Cc'ing him just in case.
> > 
> 
> We should have changelogged these considerations.
> 
> I've asked Joe if checkpatch can get a "default y" detector, to draw
> attention to this in the future.

Perhaps:
---
 scripts/checkpatch.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bf7c5abeefaad..1abec0cd217e6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3523,6 +3523,19 @@ sub process {
 			}
 		}
 
+# check new Kconfig entries for default=y
+# Only applies when adding the entry originally and if the entry is not
+# conditional on anything like default y if <foo>
+		if ($realfile =~ /Kconfig/ &&
+		    !$file &&
+		    $line =~ /\+\s*default\s+y\s*$/i) {
+			if (WARN("KCONFIG_DEFAULT_Y",
+				 "Kconfig entries should generally not be default y\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$fixlinenr] =~ s/default(\s+)y/default${1}n/;
+			}
+		}
+
 # check MAINTAINERS entries
 		if ($realfile =~ /^MAINTAINERS$/) {
 # check MAINTAINERS entries for the right form