linux-kernel - Re: [PATCH v2] printk: Remove redundant CONFIG_BASE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK7LNAR=YgBR=FYeZ+HKuFYOP3ad0K0tmqtsuHD6N-cHW5QoNQ@mail.gmail.com>
Date: Wed, 31 Jan 2024 23:34:26 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Luis Chamberlain <mcgrof@...nel.org>
Cc: Yoann Congal <yoann.congal@...le.fr>, Josh Triplett <josh@...htriplett.org>, 
	Petr Mladek <pmladek@...e.com>, Steven Rostedt <rostedt@...dmis.org>, 
	John Ogness <john.ogness@...utronix.de>, Sergey Senozhatsky <senozhatsky@...omium.org>, x86@...nel.org, 
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-kbuild@...r.kernel.org, 
	Geert Uytterhoeven <geert@...ux-m68k.org>, Thomas Gleixner <tglx@...utronix.de>, 
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>, 
	Willem de Bruijn <willemdebruijn.kernel@...il.com>, Matthew Wilcox <willy@...radead.org>, 
	Peter Zijlstra <peterz@...radead.org>, Darren Hart <dvhart@...radead.org>, 
	Davidlohr Bueso <dave@...olabs.net>, André Almeida <andrealmeid@...lia.com>
Subject: Re: [PATCH v2] printk: Remove redundant CONFIG_BASE_SMALL

On Tue, Jan 30, 2024 at 1:00 AM Luis Chamberlain <mcgrof@...nel.org> wrote:
>
> You wanna address the printk maintainers, which I've added now.
> And Josh as he's interested in tiny linux.
>
> On Sat, Jan 27, 2024 at 11:00:26PM +0100, Yoann Congal wrote:
> > CONFIG_BASE_SMALL is currently a type int but is only used as a boolean
> > equivalent to !CONFIG_BASE_FULL.
> >
> > So, remove it entirely and move every usage to !CONFIG_BASE_FULL.
>
> Thanks for doing this.
>
> > In addition, recent kconfig changes (see the discussion in Closes: tag)
> > revealed that using:
> >   config SOMETHING
> >      default "some value" if X
> > does not work as expected if X is not of type bool.
>
> We should see if we can get kconfig to warn on this type of use.
> Also note that this was reported long ago by Vegard Nossum but he
> never really sent a fix [0] as I suggested, so thanks for doing this
> work.
>
> [0] https://lkml.iu.edu/hypermail/linux/kernel/2110.2/02402.html



It is good to know that this issue was already pointed out
in the past.



> You should mention the one case which this patch fixes is:
>
> > CONFIG_BASE_SMALL was used that way in init/Kconfig:
> >   config LOG_CPU_MAX_BUF_SHIFT
> >       default 12 if !BASE_SMALL
> >       default 0 if BASE_SMALL
>
> You should then mention this has been using 12 for a long time now
> for BASE_SMALL, and so this patch is a functional fix for those
> who used BASE_SMALL and wanted a smaller printk buffer contribtion per
> cpu. The contribution was only per CPU, and since BASE_SMALL systems
> likely don't have many CPUs the impact of this was relatively small,
> 4 KiB per CPU.  This patch fixes that back down to 0 KiB per CPU.
>
> So in practice I'd imagine this fix is not critical to stable. However
> if folks do want it backported I'll note BAS_FULL has been around since
> we started with git on Linux so it should backport just fine.
>
> > diff --git a/init/Kconfig b/init/Kconfig
> > index 8d4e836e1b6b1..877b3f6f0e605 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -734,8 +734,8 @@ config LOG_CPU_MAX_BUF_SHIFT
> >       int "CPU kernel log buffer size contribution (13 => 8 KB, 17 => 128KB)"
> >       depends on SMP
> >       range 0 21
> > -     default 12 if !BASE_SMALL
> > -     default 0 if BASE_SMALL
> > +     default 12 if BASE_FULL
> > +     default 0
> >       depends on PRINTK
> >       help
> >         This option allows to increase the default ring buffer size
>
> This is the only functional change, it is a fix, so please address
> this in a separate small patch where you can go into all the above
> details about its issue and implications of fixing this as per my
> note above.
>
> Then you can address a separate patch which addresses the move of
> BASE_SMALL users to BASE_FULL so to remove BASE_SMALL, that is
> because that commit would have no functional changes and it makes
> it easier to review.
>
>   Luis



Splitting this into two patches sounds fine to me.
Either is fine. Up to the printk maintainer.

Anyway, this patch looks good:

Reviewed-by: Masahiro Yamada <masahiroy@...nel.org>







-- 
Best Regards
Masahiro Yamada