On Tue, 18 Aug 2020 18:16:26 +0300 Mike Rapoport <rppt@xxxxxxxxxx> wrote: > From: Mike Rapoport <rppt@xxxxxxxxxxxxx> > > The only user of memblock_dbg() outside memblock was s390 setup code and it > is converted to use pr_debug() instead. > This allows to stop exposing memblock_debug and memblock_dbg() to the rest > of the kernel. > > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -137,7 +137,10 @@ struct memblock_type physmem = { > i < memblock_type->cnt; \ > i++, rgn = &memblock_type->regions[i]) > > -int memblock_debug __initdata_memblock; > +#define memblock_dbg(fmt, ...) \ > + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) > + checkpatch doesn't like this much. ERROR: Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects #101: FILE: mm/memblock.c:140: +#define memblock_dbg(fmt, ...) \ + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ... #102: FILE: mm/memblock.c:141: + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) ERROR: trailing statements should be on next line #102: FILE: mm/memblock.c:141: + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) The first one is significant: if (foo) memblock_dbg(...); else save_the_world(); could end up inadvertently destroying the planet. This? --- a/mm/memblock.c~memblock-make-memblock_debug-and-related-functionality-private-fix +++ a/mm/memblock.c @@ -137,8 +137,11 @@ struct memblock_type physmem = { i < memblock_type->cnt; \ i++, rgn = &memblock_type->regions[i]) -#define memblock_dbg(fmt, ...) \ - if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) +#define memblock_dbg(fmt, ...) \ + do { \ + if (memblock_debug) \ + pr_info(fmt, ##__VA_ARGS__); \ + } while (0) static int memblock_debug __initdata_memblock; static bool system_has_some_mirror __initdata_memblock = false; _