On Tue, Jan 30, 2024 at 1:00 AM Luis Chamberlain <mcgrof@xxxxxxxxxx> 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@xxxxxxxxxx> -- Best Regards Masahiro Yamada
