| Message ID | 20200818151634.14343-10-rppt@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | memblock: seasonal cleaning^w cleanup | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (d4ecce4dcc8f8820286cf4e0859850c555e89854) |
| snowpatch_ozlabs/checkpatch | warning | total: 2 errors, 1 warnings, 0 checks, 71 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On Tue, 18 Aug 2020 18:16:26 +0300 Mike Rapoport <rppt@kernel.org> wrote: > From: Mike Rapoport <rppt@linux.ibm.com> > > 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;
On Wed, Aug 19, 2020 at 12:24:05PM -0700, Andrew Morton wrote: > On Tue, 18 Aug 2020 18:16:26 +0300 Mike Rapoport <rppt@kernel.org> wrote: > > > From: Mike Rapoport <rppt@linux.ibm.com> > > > > 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. Well, it didn't till now ;-) > 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) Sure, thanks! > static int memblock_debug __initdata_memblock; > static bool system_has_some_mirror __initdata_memblock = false; > _ >
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c index e600f6953d7c..68089eabae27 100644 --- a/arch/s390/kernel/setup.c +++ b/arch/s390/kernel/setup.c @@ -776,8 +776,8 @@ static void __init memblock_add_mem_detect_info(void) unsigned long start, end; int i; - memblock_dbg("physmem info source: %s (%hhd)\n", - get_mem_info_source(), mem_detect.info_source); + pr_debug("physmem info source: %s (%hhd)\n", + get_mem_info_source(), mem_detect.info_source); /* keep memblock lists close to the kernel */ memblock_set_bottom_up(true); for_each_mem_detect_block(i, &start, &end) { diff --git a/include/linux/memblock.h b/include/linux/memblock.h index 550faf69fc1c..47a76e237fca 100644 --- a/include/linux/memblock.h +++ b/include/linux/memblock.h @@ -86,7 +86,6 @@ struct memblock { }; extern struct memblock memblock; -extern int memblock_debug; #ifndef CONFIG_ARCH_KEEP_MEMBLOCK #define __init_memblock __meminit @@ -98,9 +97,6 @@ void memblock_discard(void); static inline void memblock_discard(void) {} #endif -#define memblock_dbg(fmt, ...) \ - if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) - phys_addr_t memblock_find_in_range(phys_addr_t start, phys_addr_t end, phys_addr_t size, phys_addr_t align); void memblock_allow_resize(void); @@ -476,13 +472,7 @@ bool memblock_is_region_memory(phys_addr_t base, phys_addr_t size); bool memblock_is_reserved(phys_addr_t addr); bool memblock_is_region_reserved(phys_addr_t base, phys_addr_t size); -extern void __memblock_dump_all(void); - -static inline void memblock_dump_all(void) -{ - if (memblock_debug) - __memblock_dump_all(); -} +void memblock_dump_all(void); /** * memblock_set_current_limit - Set the current allocation limit to allow diff --git a/mm/memblock.c b/mm/memblock.c index 59f3998ae5db..799513f3d6a9 100644 --- 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__) + +static int memblock_debug __initdata_memblock; static bool system_has_some_mirror __initdata_memblock = false; static int memblock_can_resize __initdata_memblock; static int memblock_memory_in_slab __initdata_memblock = 0; @@ -1920,7 +1923,7 @@ static void __init_memblock memblock_dump(struct memblock_type *type) } } -void __init_memblock __memblock_dump_all(void) +static void __init_memblock __memblock_dump_all(void) { pr_info("MEMBLOCK configuration:\n"); pr_info(" memory size = %pa reserved size = %pa\n", @@ -1934,6 +1937,12 @@ void __init_memblock __memblock_dump_all(void) #endif } +void __init_memblock memblock_dump_all(void) +{ + if (memblock_debug) + __memblock_dump_all(); +} + void __init memblock_allow_resize(void) { memblock_can_resize = 1;