2020-03-06 16:03:16

by Jessica Yu

[permalink] [raw]
Subject: [PATCH v3 1/2] modpost: rework and consolidate logging interface

Rework modpost's logging interface by consolidating merror(), warn(), and
fatal() to use a single function, modpost_log(). Introduce different
logging levels (WARN, ERROR, FATAL) as well. The purpose of this cleanup is
to reduce code duplication when deciding whether or not to warn or error
out based on a condition.

Signed-off-by: Jessica Yu <[email protected]>
---
v3:
- remove level variable from modpost_log and just call fprintf in each
case
- remove warn_unless and just call modpost_log() directly
- fix checkpatch error:
ERROR: space required before the open parenthesis '('
#102: FILE: scripts/mod/modpost.c:61:
+ switch(loglevel) {

scripts/mod/modpost.c | 68 ++++++++++++++++++++++-----------------------------
scripts/mod/modpost.h | 14 ++++++++---
2 files changed, 40 insertions(+), 42 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 7edfdb2f4497..a2329235a6db 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -51,41 +51,34 @@ enum export {

#define MODULE_NAME_LEN (64 - sizeof(Elf_Addr))

-#define PRINTF __attribute__ ((format (printf, 1, 2)))
+#define PRINTF __attribute__ ((format (printf, 2, 3)))

-PRINTF void fatal(const char *fmt, ...)
+PRINTF void modpost_log(enum loglevel loglevel, const char *fmt, ...)
{
va_list arglist;

- fprintf(stderr, "FATAL: ");
-
- va_start(arglist, fmt);
- vfprintf(stderr, fmt, arglist);
- va_end(arglist);
-
- exit(1);
-}
-
-PRINTF void warn(const char *fmt, ...)
-{
- va_list arglist;
+ switch (loglevel) {
+ case LOG_WARN:
+ fprintf(stderr, "WARNING: ");
+ break;
+ case LOG_ERROR:
+ fprintf(stderr, "ERROR: ");
+ break;
+ case LOG_FATAL:
+ fprintf(stderr, "FATAL: ");
+ break;
+ default: /* invalid loglevel, ignore */
+ break;
+ }

- fprintf(stderr, "WARNING: ");
+ fprintf(stderr, "modpost: ");

va_start(arglist, fmt);
vfprintf(stderr, fmt, arglist);
va_end(arglist);
-}
-
-PRINTF void merror(const char *fmt, ...)
-{
- va_list arglist;

- fprintf(stderr, "ERROR: ");
-
- va_start(arglist, fmt);
- vfprintf(stderr, fmt, arglist);
- va_end(arglist);
+ if (loglevel == LOG_FATAL)
+ exit(1);
}

static inline bool strends(const char *str, const char *postfix)
@@ -113,7 +106,7 @@ static int is_vmlinux(const char *modname)
void *do_nofail(void *ptr, const char *expr)
{
if (!ptr)
- fatal("modpost: Memory allocation failure: %s.\n", expr);
+ fatal("Memory allocation failure: %s.\n", expr);

return ptr;
}
@@ -2021,7 +2014,7 @@ static void read_symbols(const char *modname)

license = get_modinfo(&info, "license");
if (!license && !is_vmlinux(modname))
- warn("modpost: missing MODULE_LICENSE() in %s\n"
+ warn("missing MODULE_LICENSE() in %s\n"
"see include/linux/module.h for "
"more information\n", modname);
while (license) {
@@ -2152,15 +2145,15 @@ static void check_for_gpl_usage(enum export exp, const char *m, const char *s)

switch (exp) {
case export_gpl:
- fatal("modpost: GPL-incompatible module %s%s "
+ fatal("GPL-incompatible module %s%s "
"uses GPL-only symbol '%s'\n", m, e, s);
break;
case export_unused_gpl:
- fatal("modpost: GPL-incompatible module %s%s "
+ fatal("GPL-incompatible module %s%s "
"uses GPL-only symbol marked UNUSED '%s'\n", m, e, s);
break;
case export_gpl_future:
- warn("modpost: GPL-incompatible module %s%s "
+ warn("GPL-incompatible module %s%s "
"uses future GPL-only symbol '%s'\n", m, e, s);
break;
case export_plain:
@@ -2178,7 +2171,7 @@ static void check_for_unused(enum export exp, const char *m, const char *s)
switch (exp) {
case export_unused:
case export_unused_gpl:
- warn("modpost: module %s%s "
+ warn("module %s%s "
"uses symbol '%s' marked UNUSED\n", m, e, s);
break;
default:
@@ -2197,14 +2190,11 @@ static int check_exports(struct module *mod)
exp = find_symbol(s->name);
if (!exp || exp->module == mod) {
if (have_vmlinux && !s->weak) {
- if (warn_unresolved) {
- warn("\"%s\" [%s.ko] undefined!\n",
- s->name, mod->name);
- } else {
- merror("\"%s\" [%s.ko] undefined!\n",
- s->name, mod->name);
+ modpost_log(warn_unresolved ? LOG_WARN : LOG_ERROR,
+ "\"%s\" [%s.ko] undefined!\n",
+ s->name, mod->name);
+ if (!warn_unresolved)
err = 1;
- }
}
continue;
}
@@ -2653,7 +2643,7 @@ int main(int argc, char **argv)
if (dump_write)
write_dump(dump_write);
if (sec_mismatch_count && sec_mismatch_fatal)
- fatal("modpost: Section mismatches detected.\n"
+ fatal("Section mismatches detected.\n"
"Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
for (n = 0; n < SYMBOL_HASH_SIZE; n++) {
struct symbol *s;
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 64a82d2d85f6..60dca9b7106b 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -198,6 +198,14 @@ void *grab_file(const char *filename, unsigned long *size);
char* get_next_line(unsigned long *pos, void *file, unsigned long size);
void release_file(void *file, unsigned long size);

-void fatal(const char *fmt, ...);
-void warn(const char *fmt, ...);
-void merror(const char *fmt, ...);
+enum loglevel {
+ LOG_WARN,
+ LOG_ERROR,
+ LOG_FATAL
+};
+
+void modpost_log(enum loglevel loglevel, const char *fmt, ...);
+
+#define warn(fmt, args...) modpost_log(LOG_WARN, fmt, ##args)
+#define merror(fmt, args...) modpost_log(LOG_ERROR, fmt, ##args)
+#define fatal(fmt, args...) modpost_log(LOG_FATAL, fmt, ##args)
--
2.16.4


2020-03-06 16:05:43

by Jessica Yu

[permalink] [raw]
Subject: [PATCH v3 2/2] modpost: return error if module is missing ns imports and MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n

Currently when CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n, modpost
only warns when a module is missing namespace imports. Under this
configuration, such a module cannot be loaded into the kernel anyway, as
the module loader would reject it. We might as well return a build
error when a module is missing namespace imports under
CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n, so that the build
warning does not go ignored/unnoticed.

Signed-off-by: Jessica Yu <[email protected]>
---
v3:
- fix checkpatch errors "do not initialise statics to 0" and "possible
unwrapped commit description"

scripts/Makefile.modpost | 15 ++++++++-------
scripts/mod/modpost.c | 14 +++++++++++---
2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index b4d3f2d122ac..957eed6a17a5 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -46,13 +46,14 @@ include scripts/Kbuild.include
kernelsymfile := $(objtree)/Module.symvers
modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers

-MODPOST = scripts/mod/modpost \
- $(if $(CONFIG_MODVERSIONS),-m) \
- $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a) \
- $(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
- $(if $(KBUILD_EXTMOD),$(addprefix -e ,$(KBUILD_EXTRA_SYMBOLS))) \
- $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
- $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
+MODPOST = scripts/mod/modpost \
+ $(if $(CONFIG_MODVERSIONS),-m) \
+ $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a) \
+ $(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
+ $(if $(KBUILD_EXTMOD),$(addprefix -e ,$(KBUILD_EXTRA_SYMBOLS))) \
+ $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
+ $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
+ $(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N) \
$(if $(KBUILD_MODPOST_WARN),-w)

ifdef MODPOST_VMLINUX
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index a2329235a6db..036cc3bcb3e3 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -39,6 +39,8 @@ static int sec_mismatch_count = 0;
static int sec_mismatch_fatal = 0;
/* ignore missing files */
static int ignore_missing_files;
+/* If set to 1, only warn (instead of error) about missing ns imports */
+static int allow_missing_ns_imports;

enum export {
export_plain, export_unused, export_gpl,
@@ -2206,8 +2208,11 @@ static int check_exports(struct module *mod)

if (exp->namespace &&
!module_imports_namespace(mod, exp->namespace)) {
- warn("module %s uses symbol %s from namespace %s, but does not import it.\n",
- basename, exp->name, exp->namespace);
+ modpost_log(allow_missing_ns_imports ? LOG_WARN : LOG_ERROR,
+ "module %s uses symbol %s from namespace %s, but does not import it.\n",
+ basename, exp->name, exp->namespace);
+ if (!allow_missing_ns_imports)
+ err = 1;
add_namespace(&mod->missing_namespaces, exp->namespace);
}

@@ -2550,7 +2555,7 @@ int main(int argc, char **argv)
struct ext_sym_list *extsym_iter;
struct ext_sym_list *extsym_start = NULL;

- while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd:")) != -1) {
+ while ((opt = getopt(argc, argv, "i:e:mnsT:o:awENd:")) != -1) {
switch (opt) {
case 'i':
kernel_read = optarg;
@@ -2588,6 +2593,9 @@ int main(int argc, char **argv)
case 'E':
sec_mismatch_fatal = 1;
break;
+ case 'N':
+ allow_missing_ns_imports = 1;
+ break;
case 'd':
missing_namespace_deps = optarg;
break;
--
2.16.4

2020-03-09 00:45:53

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

Hi Jessica,



On Sat, Mar 7, 2020 at 1:02 AM Jessica Yu <[email protected]> wrote:
>
> Rework modpost's logging interface by consolidating merror(), warn(), and
> fatal() to use a single function, modpost_log(). Introduce different
> logging levels (WARN, ERROR, FATAL) as well. The purpose of this cleanup is
> to reduce code duplication when deciding whether or not to warn or error
> out based on a condition.
>
> Signed-off-by: Jessica Yu <[email protected]>
> ---
> v3:
> - remove level variable from modpost_log and just call fprintf in each
> case
> - remove warn_unless and just call modpost_log() directly
> - fix checkpatch error:
> ERROR: space required before the open parenthesis '('
> #102: FILE: scripts/mod/modpost.c:61:
> + switch(loglevel) {
>
> scripts/mod/modpost.c | 68 ++++++++++++++++++++++-----------------------------
> scripts/mod/modpost.h | 14 ++++++++---
> 2 files changed, 40 insertions(+), 42 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 7edfdb2f4497..a2329235a6db 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -51,41 +51,34 @@ enum export {
>
> #define MODULE_NAME_LEN (64 - sizeof(Elf_Addr))
>
> -#define PRINTF __attribute__ ((format (printf, 1, 2)))
> +#define PRINTF __attribute__ ((format (printf, 2, 3)))
>
> -PRINTF void fatal(const char *fmt, ...)
> +PRINTF void modpost_log(enum loglevel loglevel, const char *fmt, ...)
> {


This series looks good to me.

I can queue it up to kbuild tree
if there is no objection.


I just noticed one nit.

Now that modpost_log() is the only user of PRINTF,
we can delete PRITNF, and directly add the attribute
to modpost_log(), like this:


void __attribute__((format(printf, 2, 3)))
modpost_log(enum loglevel loglevel, const char *fmt, ...)
{
...
}


If you agree, I can modify it when I apply it.


Thank you.



--
Best Regards
Masahiro Yamada

2020-03-09 09:59:53

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

+++ Masahiro Yamada [09/03/20 09:40 +0900]:
>Hi Jessica,
>
>
>
>On Sat, Mar 7, 2020 at 1:02 AM Jessica Yu <[email protected]> wrote:
>>
>> Rework modpost's logging interface by consolidating merror(), warn(), and
>> fatal() to use a single function, modpost_log(). Introduce different
>> logging levels (WARN, ERROR, FATAL) as well. The purpose of this cleanup is
>> to reduce code duplication when deciding whether or not to warn or error
>> out based on a condition.
>>
>> Signed-off-by: Jessica Yu <[email protected]>
>> ---
>> v3:
>> - remove level variable from modpost_log and just call fprintf in each
>> case
>> - remove warn_unless and just call modpost_log() directly
>> - fix checkpatch error:
>> ERROR: space required before the open parenthesis '('
>> #102: FILE: scripts/mod/modpost.c:61:
>> + switch(loglevel) {
>>
>> scripts/mod/modpost.c | 68 ++++++++++++++++++++++-----------------------------
>> scripts/mod/modpost.h | 14 ++++++++---
>> 2 files changed, 40 insertions(+), 42 deletions(-)
>>
>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>> index 7edfdb2f4497..a2329235a6db 100644
>> --- a/scripts/mod/modpost.c
>> +++ b/scripts/mod/modpost.c
>> @@ -51,41 +51,34 @@ enum export {
>>
>> #define MODULE_NAME_LEN (64 - sizeof(Elf_Addr))
>>
>> -#define PRINTF __attribute__ ((format (printf, 1, 2)))
>> +#define PRINTF __attribute__ ((format (printf, 2, 3)))
>>
>> -PRINTF void fatal(const char *fmt, ...)
>> +PRINTF void modpost_log(enum loglevel loglevel, const char *fmt, ...)
>> {
>
>
>This series looks good to me.
>
>I can queue it up to kbuild tree
>if there is no objection.
>
>
>I just noticed one nit.
>
>Now that modpost_log() is the only user of PRINTF,
>we can delete PRITNF, and directly add the attribute
>to modpost_log(), like this:
>
>
>void __attribute__((format(printf, 2, 3)))
>modpost_log(enum loglevel loglevel, const char *fmt, ...)
>{
> ...
>}
>
>
>If you agree, I can modify it when I apply it.

Yes, I agree with this change. Thank you!

One more thing, it's not immediately obvious to me why the first patch
would cause those kbuild warnings :-/ I'll see if I have any luck
reproducing them locally..

Thanks,

Jessica

2020-03-09 10:22:09

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

On Mon, Mar 9, 2020 at 6:59 PM Jessica Yu <[email protected]> wrote:
>
> +++ Masahiro Yamada [09/03/20 09:40 +0900]:
> >Hi Jessica,
> >
> >
> >
> >On Sat, Mar 7, 2020 at 1:02 AM Jessica Yu <[email protected]> wrote:
> >>
> >> Rework modpost's logging interface by consolidating merror(), warn(), and
> >> fatal() to use a single function, modpost_log(). Introduce different
> >> logging levels (WARN, ERROR, FATAL) as well. The purpose of this cleanup is
> >> to reduce code duplication when deciding whether or not to warn or error
> >> out based on a condition.
> >>
> >> Signed-off-by: Jessica Yu <[email protected]>
> >> ---
> >> v3:
> >> - remove level variable from modpost_log and just call fprintf in each
> >> case
> >> - remove warn_unless and just call modpost_log() directly
> >> - fix checkpatch error:
> >> ERROR: space required before the open parenthesis '('
> >> #102: FILE: scripts/mod/modpost.c:61:
> >> + switch(loglevel) {
> >>
> >> scripts/mod/modpost.c | 68 ++++++++++++++++++++++-----------------------------
> >> scripts/mod/modpost.h | 14 ++++++++---
> >> 2 files changed, 40 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> >> index 7edfdb2f4497..a2329235a6db 100644
> >> --- a/scripts/mod/modpost.c
> >> +++ b/scripts/mod/modpost.c
> >> @@ -51,41 +51,34 @@ enum export {
> >>
> >> #define MODULE_NAME_LEN (64 - sizeof(Elf_Addr))
> >>
> >> -#define PRINTF __attribute__ ((format (printf, 1, 2)))
> >> +#define PRINTF __attribute__ ((format (printf, 2, 3)))
> >>
> >> -PRINTF void fatal(const char *fmt, ...)
> >> +PRINTF void modpost_log(enum loglevel loglevel, const char *fmt, ...)
> >> {
> >
> >
> >This series looks good to me.
> >
> >I can queue it up to kbuild tree
> >if there is no objection.
> >
> >
> >I just noticed one nit.
> >
> >Now that modpost_log() is the only user of PRINTF,
> >we can delete PRITNF, and directly add the attribute
> >to modpost_log(), like this:
> >
> >
> >void __attribute__((format(printf, 2, 3)))
> >modpost_log(enum loglevel loglevel, const char *fmt, ...)
> >{
> > ...
> >}
> >
> >
> >If you agree, I can modify it when I apply it.
>
> Yes, I agree with this change. Thank you!
>
> One more thing, it's not immediately obvious to me why the first patch
> would cause those kbuild warnings :-/ I'll see if I have any luck
> reproducing them locally..
>


Hmm, this warning option is not so new.


At least, commit 7c0d35a339db6 (4 years ago)
fixed a similar one.



I use GCC 7.4

If I apply your previous version,
I see build log like follows:




$ gcc --version
gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

masahiro@pug:~/ref/linux$ make defconfig all
HOSTCC scripts/kconfig/conf.o
HOSTCC scripts/kconfig/confdata.o
HOSTCC scripts/kconfig/expr.o
LEX scripts/kconfig/lexer.lex.c
YACC scripts/kconfig/parser.tab.[ch]
HOSTCC scripts/kconfig/lexer.lex.o
HOSTCC scripts/kconfig/parser.tab.o
HOSTCC scripts/kconfig/preprocess.o
HOSTCC scripts/kconfig/symbol.o
HOSTCC scripts/kconfig/util.o
HOSTLD scripts/kconfig/conf
*** Default configuration is based on 'x86_64_defconfig'
#
# configuration written to .config
#
SYSTBL arch/x86/include/generated/asm/syscalls_32.h
SYSHDR arch/x86/include/generated/asm/unistd_32_ia32.h
SYSHDR arch/x86/include/generated/asm/unistd_64_x32.h
SYSTBL arch/x86/include/generated/asm/syscalls_64.h
SYSHDR arch/x86/include/generated/uapi/asm/unistd_32.h
SYSHDR arch/x86/include/generated/uapi/asm/unistd_64.h
SYSHDR arch/x86/include/generated/uapi/asm/unistd_x32.h
HOSTCC arch/x86/tools/relocs_32.o
HOSTCC arch/x86/tools/relocs_64.o
HOSTCC arch/x86/tools/relocs_common.o
HOSTLD arch/x86/tools/relocs
HOSTCC scripts/selinux/genheaders/genheaders
HOSTCC scripts/selinux/mdp/mdp
HOSTCC scripts/kallsyms
HOSTCC scripts/sorttable
HOSTCC scripts/asn1_compiler
HOSTCC scripts/extract-cert
WRAP arch/x86/include/generated/uapi/asm/bpf_perf_event.h
WRAP arch/x86/include/generated/uapi/asm/errno.h
WRAP arch/x86/include/generated/uapi/asm/fcntl.h
WRAP arch/x86/include/generated/uapi/asm/ioctl.h
WRAP arch/x86/include/generated/uapi/asm/ioctls.h
WRAP arch/x86/include/generated/uapi/asm/ipcbuf.h
WRAP arch/x86/include/generated/uapi/asm/param.h
WRAP arch/x86/include/generated/uapi/asm/poll.h
WRAP arch/x86/include/generated/uapi/asm/resource.h
WRAP arch/x86/include/generated/uapi/asm/socket.h
WRAP arch/x86/include/generated/uapi/asm/sockios.h
WRAP arch/x86/include/generated/uapi/asm/termbits.h
WRAP arch/x86/include/generated/uapi/asm/termios.h
WRAP arch/x86/include/generated/uapi/asm/types.h
WRAP arch/x86/include/generated/asm/early_ioremap.h
WRAP arch/x86/include/generated/asm/export.h
WRAP arch/x86/include/generated/asm/mcs_spinlock.h
WRAP arch/x86/include/generated/asm/mm-arch-hooks.h
WRAP arch/x86/include/generated/asm/mmiowb.h
WRAP arch/x86/include/generated/asm/dma-contiguous.h
UPD include/config/kernel.release
UPD include/generated/uapi/linux/version.h
UPD include/generated/utsrelease.h
CC scripts/mod/empty.o
HOSTCC scripts/mod/mk_elfconfig
MKELF scripts/mod/elfconfig.h
HOSTCC scripts/mod/modpost.o
scripts/mod/modpost.c: In function ‘modpost_log’:
scripts/mod/modpost.c:75:2: warning: format not a string literal and
no format arguments [-Wformat-security]
fprintf(stderr, level);
^~~~~~~
CC scripts/mod/devicetable-offsets.s
UPD scripts/mod/devicetable-offsets.h
HOSTCC scripts/mod/file2alias.o
HOSTCC scripts/mod/sumversion.o
HOSTLD scripts/mod/modpost
CC kernel/bounds.s
UPD include/generated/bounds.h
UPD include/generated/timeconst.h
CC arch/x86/kernel/asm-offsets.s


--
Best Regards
Masahiro Yamada

2020-03-09 10:40:16

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

+++ Masahiro Yamada [09/03/20 19:20 +0900]:
>On Mon, Mar 9, 2020 at 6:59 PM Jessica Yu <[email protected]> wrote:
>>
>> +++ Masahiro Yamada [09/03/20 09:40 +0900]:
>> >Hi Jessica,
>> >
>> >
>> >
>> >On Sat, Mar 7, 2020 at 1:02 AM Jessica Yu <[email protected]> wrote:
>> >>
>> >> Rework modpost's logging interface by consolidating merror(), warn(), and
>> >> fatal() to use a single function, modpost_log(). Introduce different
>> >> logging levels (WARN, ERROR, FATAL) as well. The purpose of this cleanup is
>> >> to reduce code duplication when deciding whether or not to warn or error
>> >> out based on a condition.
>> >>
>> >> Signed-off-by: Jessica Yu <[email protected]>
>> >> ---
>> >> v3:
>> >> - remove level variable from modpost_log and just call fprintf in each
>> >> case
>> >> - remove warn_unless and just call modpost_log() directly
>> >> - fix checkpatch error:
>> >> ERROR: space required before the open parenthesis '('
>> >> #102: FILE: scripts/mod/modpost.c:61:
>> >> + switch(loglevel) {
>> >>
>> >> scripts/mod/modpost.c | 68 ++++++++++++++++++++++-----------------------------
>> >> scripts/mod/modpost.h | 14 ++++++++---
>> >> 2 files changed, 40 insertions(+), 42 deletions(-)
>> >>
>> >> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>> >> index 7edfdb2f4497..a2329235a6db 100644
>> >> --- a/scripts/mod/modpost.c
>> >> +++ b/scripts/mod/modpost.c
>> >> @@ -51,41 +51,34 @@ enum export {
>> >>
>> >> #define MODULE_NAME_LEN (64 - sizeof(Elf_Addr))
>> >>
>> >> -#define PRINTF __attribute__ ((format (printf, 1, 2)))
>> >> +#define PRINTF __attribute__ ((format (printf, 2, 3)))
>> >>
>> >> -PRINTF void fatal(const char *fmt, ...)
>> >> +PRINTF void modpost_log(enum loglevel loglevel, const char *fmt, ...)
>> >> {
>> >
>> >
>> >This series looks good to me.
>> >
>> >I can queue it up to kbuild tree
>> >if there is no objection.
>> >
>> >
>> >I just noticed one nit.
>> >
>> >Now that modpost_log() is the only user of PRINTF,
>> >we can delete PRITNF, and directly add the attribute
>> >to modpost_log(), like this:
>> >
>> >
>> >void __attribute__((format(printf, 2, 3)))
>> >modpost_log(enum loglevel loglevel, const char *fmt, ...)
>> >{
>> > ...
>> >}
>> >
>> >
>> >If you agree, I can modify it when I apply it.
>>
>> Yes, I agree with this change. Thank you!
>>
>> One more thing, it's not immediately obvious to me why the first patch
>> would cause those kbuild warnings :-/ I'll see if I have any luck
>> reproducing them locally..
>>
>
>
>Hmm, this warning option is not so new.
>
>
>At least, commit 7c0d35a339db6 (4 years ago)
>fixed a similar one.

Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
the 0-day bot emails are not CC'd to lkml. Here is the error I got
from the bot:

---

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.6-rc4 next-20200306]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
config: sh-randconfig-a001-20200306 (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=sh

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

>> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

>
>I use GCC 7.4
>
>If I apply your previous version,
>I see build log like follows:
>
>
>
>
>$ gcc --version
>gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
>Copyright (C) 2017 Free Software Foundation, Inc.
>This is free software; see the source for copying conditions. There is NO
>warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
>masahiro@pug:~/ref/linux$ make defconfig all
> HOSTCC scripts/kconfig/conf.o
> HOSTCC scripts/kconfig/confdata.o
> HOSTCC scripts/kconfig/expr.o
> LEX scripts/kconfig/lexer.lex.c
> YACC scripts/kconfig/parser.tab.[ch]
> HOSTCC scripts/kconfig/lexer.lex.o
> HOSTCC scripts/kconfig/parser.tab.o
> HOSTCC scripts/kconfig/preprocess.o
> HOSTCC scripts/kconfig/symbol.o
> HOSTCC scripts/kconfig/util.o
> HOSTLD scripts/kconfig/conf
>*** Default configuration is based on 'x86_64_defconfig'
>#
># configuration written to .config
>#
> SYSTBL arch/x86/include/generated/asm/syscalls_32.h
> SYSHDR arch/x86/include/generated/asm/unistd_32_ia32.h
> SYSHDR arch/x86/include/generated/asm/unistd_64_x32.h
> SYSTBL arch/x86/include/generated/asm/syscalls_64.h
> SYSHDR arch/x86/include/generated/uapi/asm/unistd_32.h
> SYSHDR arch/x86/include/generated/uapi/asm/unistd_64.h
> SYSHDR arch/x86/include/generated/uapi/asm/unistd_x32.h
> HOSTCC arch/x86/tools/relocs_32.o
> HOSTCC arch/x86/tools/relocs_64.o
> HOSTCC arch/x86/tools/relocs_common.o
> HOSTLD arch/x86/tools/relocs
> HOSTCC scripts/selinux/genheaders/genheaders
> HOSTCC scripts/selinux/mdp/mdp
> HOSTCC scripts/kallsyms
> HOSTCC scripts/sorttable
> HOSTCC scripts/asn1_compiler
> HOSTCC scripts/extract-cert
> WRAP arch/x86/include/generated/uapi/asm/bpf_perf_event.h
> WRAP arch/x86/include/generated/uapi/asm/errno.h
> WRAP arch/x86/include/generated/uapi/asm/fcntl.h
> WRAP arch/x86/include/generated/uapi/asm/ioctl.h
> WRAP arch/x86/include/generated/uapi/asm/ioctls.h
> WRAP arch/x86/include/generated/uapi/asm/ipcbuf.h
> WRAP arch/x86/include/generated/uapi/asm/param.h
> WRAP arch/x86/include/generated/uapi/asm/poll.h
> WRAP arch/x86/include/generated/uapi/asm/resource.h
> WRAP arch/x86/include/generated/uapi/asm/socket.h
> WRAP arch/x86/include/generated/uapi/asm/sockios.h
> WRAP arch/x86/include/generated/uapi/asm/termbits.h
> WRAP arch/x86/include/generated/uapi/asm/termios.h
> WRAP arch/x86/include/generated/uapi/asm/types.h
> WRAP arch/x86/include/generated/asm/early_ioremap.h
> WRAP arch/x86/include/generated/asm/export.h
> WRAP arch/x86/include/generated/asm/mcs_spinlock.h
> WRAP arch/x86/include/generated/asm/mm-arch-hooks.h
> WRAP arch/x86/include/generated/asm/mmiowb.h
> WRAP arch/x86/include/generated/asm/dma-contiguous.h
> UPD include/config/kernel.release
> UPD include/generated/uapi/linux/version.h
> UPD include/generated/utsrelease.h
> CC scripts/mod/empty.o
> HOSTCC scripts/mod/mk_elfconfig
> MKELF scripts/mod/elfconfig.h
> HOSTCC scripts/mod/modpost.o
>scripts/mod/modpost.c: In function ‘modpost_log’:
>scripts/mod/modpost.c:75:2: warning: format not a string literal and
>no format arguments [-Wformat-security]
> fprintf(stderr, level);
> ^~~~~~~
> CC scripts/mod/devicetable-offsets.s
> UPD scripts/mod/devicetable-offsets.h
> HOSTCC scripts/mod/file2alias.o
> HOSTCC scripts/mod/sumversion.o
> HOSTLD scripts/mod/modpost
> CC kernel/bounds.s
> UPD include/generated/bounds.h
> UPD include/generated/timeconst.h
> CC arch/x86/kernel/asm-offsets.s
>
>
>--
>Best Regards
>Masahiro Yamada

2020-03-09 10:51:17

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
>
> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
> the 0-day bot emails are not CC'd to lkml. Here is the error I got
> from the bot:
>
> ---
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.6-rc4 next-20200306]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
> config: sh-randconfig-a001-20200306 (attached as .config)
> compiler: sh4-linux-gcc (GCC) 7.5.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.5.0 make.cross ARCH=sh
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!


Indeed, this one is odd.
I have no idea...


--
Best Regards
Masahiro Yamada

2020-03-09 10:59:04

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

+++ Masahiro Yamada [09/03/20 19:49 +0900]:
>On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
>>
>> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
>> the 0-day bot emails are not CC'd to lkml. Here is the error I got
>> from the bot:
>>
>> ---
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on linus/master]
>> [also build test ERROR on v5.6-rc4 next-20200306]
>> [if your patch is applied to the wrong git tree, please drop us a note to help
>> improve the system. BTW, we also suggest to use '--base' option to specify the
>> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>>
>> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
>> config: sh-randconfig-a001-20200306 (attached as .config)
>> compiler: sh4-linux-gcc (GCC) 7.5.0
>> reproduce:
>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # save the attached .config to linux build tree
>> GCC_VERSION=7.5.0 make.cross ARCH=sh
>>
>> If you fix the issue, kindly add following tag
>> Reported-by: kbuild test robot <[email protected]>
>>
>> All errors (new ones prefixed by >>):
>>
>> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>
>
>Indeed, this one is odd.
>I have no idea...

I've pushed the patches to a branch to let the kbuild bot run through its
build tests again, and if I have extra time today I will try to
reproduce this and let you know the results.

Thanks,

Jessica

2020-03-09 11:04:50

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

On Mon, Mar 9, 2020 at 7:58 PM Jessica Yu <[email protected]> wrote:
>
> +++ Masahiro Yamada [09/03/20 19:49 +0900]:
> >On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
> >>
> >> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
> >> the 0-day bot emails are not CC'd to lkml. Here is the error I got
> >> from the bot:
> >>
> >> ---
> >>
> >> I love your patch! Yet something to improve:
> >>
> >> [auto build test ERROR on linus/master]
> >> [also build test ERROR on v5.6-rc4 next-20200306]
> >> [if your patch is applied to the wrong git tree, please drop us a note to help
> >> improve the system. BTW, we also suggest to use '--base' option to specify the
> >> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >>
> >> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
> >> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
> >> config: sh-randconfig-a001-20200306 (attached as .config)
> >> compiler: sh4-linux-gcc (GCC) 7.5.0
> >> reproduce:
> >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >> chmod +x ~/bin/make.cross
> >> # save the attached .config to linux build tree
> >> GCC_VERSION=7.5.0 make.cross ARCH=sh
> >>
> >> If you fix the issue, kindly add following tag
> >> Reported-by: kbuild test robot <[email protected]>
> >>
> >> All errors (new ones prefixed by >>):
> >>
> >> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> >
> >
> >Indeed, this one is odd.
> >I have no idea...
>
> I've pushed the patches to a branch to let the kbuild bot run through its
> build tests again, and if I have extra time today I will try to
> reproduce this and let you know the results.
>
> Thanks,
>
> Jessica


Ah, Now I see.


Because you added "modpost:" prefix.



The previous error message:

ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!


The new error message:

ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!



So, the bot assumed it was a new error.





--
Best Regards
Masahiro Yamada

2020-03-10 11:32:55

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

+++ Masahiro Yamada [09/03/20 20:03 +0900]:
>On Mon, Mar 9, 2020 at 7:58 PM Jessica Yu <[email protected]> wrote:
>>
>> +++ Masahiro Yamada [09/03/20 19:49 +0900]:
>> >On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
>> >>
>> >> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
>> >> the 0-day bot emails are not CC'd to lkml. Here is the error I got
>> >> from the bot:
>> >>
>> >> ---
>> >>
>> >> I love your patch! Yet something to improve:
>> >>
>> >> [auto build test ERROR on linus/master]
>> >> [also build test ERROR on v5.6-rc4 next-20200306]
>> >> [if your patch is applied to the wrong git tree, please drop us a note to help
>> >> improve the system. BTW, we also suggest to use '--base' option to specify the
>> >> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>> >>
>> >> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
>> >> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
>> >> config: sh-randconfig-a001-20200306 (attached as .config)
>> >> compiler: sh4-linux-gcc (GCC) 7.5.0
>> >> reproduce:
>> >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> >> chmod +x ~/bin/make.cross
>> >> # save the attached .config to linux build tree
>> >> GCC_VERSION=7.5.0 make.cross ARCH=sh
>> >>
>> >> If you fix the issue, kindly add following tag
>> >> Reported-by: kbuild test robot <[email protected]>
>> >>
>> >> All errors (new ones prefixed by >>):
>> >>
>> >> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>> >
>> >
>> >Indeed, this one is odd.
>> >I have no idea...
>>
>> I've pushed the patches to a branch to let the kbuild bot run through its
>> build tests again, and if I have extra time today I will try to
>> reproduce this and let you know the results.
>>
>> Thanks,
>>
>> Jessica
>
>
>Ah, Now I see.
>
>
>Because you added "modpost:" prefix.
>
>
>
>The previous error message:
>
>ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>
>
>The new error message:
>
>ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>
>
>
>So, the bot assumed it was a new error.

Nice catch! Hm, I suppose we need to let the LKP folks know about the
change in error message. CC'd LKP mailing list.

Jessica

2020-03-10 15:58:17

by Philip Li

[permalink] [raw]
Subject: Re: [LKP] Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

On Tue, Mar 10, 2020 at 12:32:00PM +0100, Jessica Yu wrote:
> +++ Masahiro Yamada [09/03/20 20:03 +0900]:
> > On Mon, Mar 9, 2020 at 7:58 PM Jessica Yu <[email protected]> wrote:
> > >
> > > +++ Masahiro Yamada [09/03/20 19:49 +0900]:
> > > >On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
> > > >>
> > > >> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
> > > >> the 0-day bot emails are not CC'd to lkml. Here is the error I got
> > > >> from the bot:
> > > >>
> > > >> ---
> > > >>
> > > >> I love your patch! Yet something to improve:
> > > >>
> > > >> [auto build test ERROR on linus/master]
> > > >> [also build test ERROR on v5.6-rc4 next-20200306]
> > > >> [if your patch is applied to the wrong git tree, please drop us a note to help
> > > >> improve the system. BTW, we also suggest to use '--base' option to specify the
> > > >> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > > >>
> > > >> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
> > > >> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
> > > >> config: sh-randconfig-a001-20200306 (attached as .config)
> > > >> compiler: sh4-linux-gcc (GCC) 7.5.0
> > > >> reproduce:
> > > >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > >> chmod +x ~/bin/make.cross
> > > >> # save the attached .config to linux build tree
> > > >> GCC_VERSION=7.5.0 make.cross ARCH=sh
> > > >>
> > > >> If you fix the issue, kindly add following tag
> > > >> Reported-by: kbuild test robot <[email protected]>
> > > >>
> > > >> All errors (new ones prefixed by >>):
> > > >>
> > > >> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> > > >
> > > >
> > > >Indeed, this one is odd.
> > > >I have no idea...
> > >
> > > I've pushed the patches to a branch to let the kbuild bot run through its
> > > build tests again, and if I have extra time today I will try to
> > > reproduce this and let you know the results.
> > >
> > > Thanks,
> > >
> > > Jessica
> >
> >
> > Ah, Now I see.
> >
> >
> > Because you added "modpost:" prefix.
> >
> >
> >
> > The previous error message:
> >
> > ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> >
> >
> > The new error message:
> >
> > ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> >
> >
> >
> > So, the bot assumed it was a new error.
>
> Nice catch! Hm, I suppose we need to let the LKP folks know about the
> change in error message. CC'd LKP mailing list.
Thanks for the info, we will look into this to handle the
changed error.

>
> Jessica
> _______________________________________________
> LKP mailing list -- [email protected]
> To unsubscribe send an email to [email protected]

2020-03-11 00:59:08

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

On Tue, Mar 10, 2020 at 8:32 PM Jessica Yu <[email protected]> wrote:
>
> +++ Masahiro Yamada [09/03/20 20:03 +0900]:
> >On Mon, Mar 9, 2020 at 7:58 PM Jessica Yu <[email protected]> wrote:
> >>
> >> +++ Masahiro Yamada [09/03/20 19:49 +0900]:
> >> >On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
> >> >>
> >> >> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
> >> >> the 0-day bot emails are not CC'd to lkml. Here is the error I got
> >> >> from the bot:
> >> >>
> >> >> ---
> >> >>
> >> >> I love your patch! Yet something to improve:
> >> >>
> >> >> [auto build test ERROR on linus/master]
> >> >> [also build test ERROR on v5.6-rc4 next-20200306]
> >> >> [if your patch is applied to the wrong git tree, please drop us a note to help
> >> >> improve the system. BTW, we also suggest to use '--base' option to specify the
> >> >> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >> >>
> >> >> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
> >> >> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
> >> >> config: sh-randconfig-a001-20200306 (attached as .config)
> >> >> compiler: sh4-linux-gcc (GCC) 7.5.0
> >> >> reproduce:
> >> >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >> >> chmod +x ~/bin/make.cross
> >> >> # save the attached .config to linux build tree
> >> >> GCC_VERSION=7.5.0 make.cross ARCH=sh
> >> >>
> >> >> If you fix the issue, kindly add following tag
> >> >> Reported-by: kbuild test robot <[email protected]>
> >> >>
> >> >> All errors (new ones prefixed by >>):
> >> >>
> >> >> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> >> >
> >> >
> >> >Indeed, this one is odd.
> >> >I have no idea...
> >>
> >> I've pushed the patches to a branch to let the kbuild bot run through its
> >> build tests again, and if I have extra time today I will try to
> >> reproduce this and let you know the results.
> >>
> >> Thanks,
> >>
> >> Jessica
> >
> >
> >Ah, Now I see.
> >
> >
> >Because you added "modpost:" prefix.
> >
> >
> >
> >The previous error message:
> >
> >ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> >
> >
> >The new error message:
> >
> >ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> >
> >
> >
> >So, the bot assumed it was a new error.
>
> Nice catch! Hm, I suppose we need to let the LKP folks know about the
> change in error message. CC'd LKP mailing list.


Both applied to linux-kbuild.




--
Best Regards
Masahiro Yamada

2020-03-11 01:02:10

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [LKP] Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

On Wed, Mar 11, 2020 at 12:56 AM Philip Li <[email protected]> wrote:
>
> On Tue, Mar 10, 2020 at 12:32:00PM +0100, Jessica Yu wrote:
> > +++ Masahiro Yamada [09/03/20 20:03 +0900]:
> > > On Mon, Mar 9, 2020 at 7:58 PM Jessica Yu <[email protected]> wrote:
> > > >
> > > > +++ Masahiro Yamada [09/03/20 19:49 +0900]:
> > > > >On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
> > > > >>
> > > > >> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
> > > > >> the 0-day bot emails are not CC'd to lkml. Here is the error I got
> > > > >> from the bot:
> > > > >>
> > > > >> ---
> > > > >>
> > > > >> I love your patch! Yet something to improve:
> > > > >>
> > > > >> [auto build test ERROR on linus/master]
> > > > >> [also build test ERROR on v5.6-rc4 next-20200306]
> > > > >> [if your patch is applied to the wrong git tree, please drop us a note to help
> > > > >> improve the system. BTW, we also suggest to use '--base' option to specify the
> > > > >> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > > > >>
> > > > >> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
> > > > >> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
> > > > >> config: sh-randconfig-a001-20200306 (attached as .config)
> > > > >> compiler: sh4-linux-gcc (GCC) 7.5.0
> > > > >> reproduce:
> > > > >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > > >> chmod +x ~/bin/make.cross
> > > > >> # save the attached .config to linux build tree
> > > > >> GCC_VERSION=7.5.0 make.cross ARCH=sh
> > > > >>
> > > > >> If you fix the issue, kindly add following tag
> > > > >> Reported-by: kbuild test robot <[email protected]>
> > > > >>
> > > > >> All errors (new ones prefixed by >>):
> > > > >>
> > > > >> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> > > > >
> > > > >
> > > > >Indeed, this one is odd.
> > > > >I have no idea...
> > > >
> > > > I've pushed the patches to a branch to let the kbuild bot run through its
> > > > build tests again, and if I have extra time today I will try to
> > > > reproduce this and let you know the results.
> > > >
> > > > Thanks,
> > > >
> > > > Jessica
> > >
> > >
> > > Ah, Now I see.
> > >
> > >
> > > Because you added "modpost:" prefix.
> > >
> > >
> > >
> > > The previous error message:
> > >
> > > ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> > >
> > >
> > > The new error message:
> > >
> > > ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> > >
> > >
> > >
> > > So, the bot assumed it was a new error.
> >
> > Nice catch! Hm, I suppose we need to let the LKP folks know about the
> > change in error message. CC'd LKP mailing list.
> Thanks for the info, we will look into this to handle the
> changed error.
>
> >
> > Jessica
> > _______________________________________________
> > LKP mailing list -- [email protected]
> > To unsubscribe send an email to [email protected]



Could you improve the report by adding more context?

Currently, only new errors/warnings are shown by '>>'.


If fixed ones had been shown by '<<',
we would have easily noticed that
this was just a matter of message format.

<< ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!




--
Best Regards
Masahiro Yamada

2020-03-11 12:28:30

by Philip Li

[permalink] [raw]
Subject: Re: [LKP] Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface

On Wed, Mar 11, 2020 at 10:00:11AM +0900, Masahiro Yamada wrote:
> On Wed, Mar 11, 2020 at 12:56 AM Philip Li <[email protected]> wrote:
> >
> > On Tue, Mar 10, 2020 at 12:32:00PM +0100, Jessica Yu wrote:
> > > +++ Masahiro Yamada [09/03/20 20:03 +0900]:
> > > > On Mon, Mar 9, 2020 at 7:58 PM Jessica Yu <[email protected]> wrote:
> > > > >
> > > > > +++ Masahiro Yamada [09/03/20 19:49 +0900]:
> > > > > >On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
> > > > > >>
> > > > > >> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
> > > > > >> the 0-day bot emails are not CC'd to lkml. Here is the error I got
> > > > > >> from the bot:
> > > > > >>
> > > > > >> ---
> > > > > >>
> > > > > >> I love your patch! Yet something to improve:
> > > > > >>
> > > > > >> [auto build test ERROR on linus/master]
> > > > > >> [also build test ERROR on v5.6-rc4 next-20200306]
> > > > > >> [if your patch is applied to the wrong git tree, please drop us a note to help
> > > > > >> improve the system. BTW, we also suggest to use '--base' option to specify the
> > > > > >> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > > > > >>
> > > > > >> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
> > > > > >> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
> > > > > >> config: sh-randconfig-a001-20200306 (attached as .config)
> > > > > >> compiler: sh4-linux-gcc (GCC) 7.5.0
> > > > > >> reproduce:
> > > > > >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > > > >> chmod +x ~/bin/make.cross
> > > > > >> # save the attached .config to linux build tree
> > > > > >> GCC_VERSION=7.5.0 make.cross ARCH=sh
> > > > > >>
> > > > > >> If you fix the issue, kindly add following tag
> > > > > >> Reported-by: kbuild test robot <[email protected]>
> > > > > >>
> > > > > >> All errors (new ones prefixed by >>):
> > > > > >>
> > > > > >> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> > > > > >
> > > > > >
> > > > > >Indeed, this one is odd.
> > > > > >I have no idea...
> > > > >
> > > > > I've pushed the patches to a branch to let the kbuild bot run through its
> > > > > build tests again, and if I have extra time today I will try to
> > > > > reproduce this and let you know the results.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jessica
> > > >
> > > >
> > > > Ah, Now I see.
> > > >
> > > >
> > > > Because you added "modpost:" prefix.
> > > >
> > > >
> > > >
> > > > The previous error message:
> > > >
> > > > ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> > > >
> > > >
> > > > The new error message:
> > > >
> > > > ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> > > >
> > > >
> > > >
> > > > So, the bot assumed it was a new error.
> > >
> > > Nice catch! Hm, I suppose we need to let the LKP folks know about the
> > > change in error message. CC'd LKP mailing list.
> > Thanks for the info, we will look into this to handle the
> > changed error.
> >
> > >
> > > Jessica
> > > _______________________________________________
> > > LKP mailing list -- [email protected]
> > > To unsubscribe send an email to [email protected]
>
>
>
> Could you improve the report by adding more context?
>
> Currently, only new errors/warnings are shown by '>>'.
>
>
> If fixed ones had been shown by '<<',
> we would have easily noticed that
> this was just a matter of message format.
thanks for the suggestion, we will think of this, though
it may not be a quick implemention for current code base :-)

>
> << ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
> >> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>
>
>
>
> --
> Best Regards
> Masahiro Yamada

2020-03-12 00:52:21

by Chen, Rong A

[permalink] [raw]
Subject: Re: [LKP] Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface



On 3/11/20 9:00 AM, Masahiro Yamada wrote:
> On Wed, Mar 11, 2020 at 12:56 AM Philip Li <[email protected]> wrote:
>> On Tue, Mar 10, 2020 at 12:32:00PM +0100, Jessica Yu wrote:
>>> +++ Masahiro Yamada [09/03/20 20:03 +0900]:
>>>> On Mon, Mar 9, 2020 at 7:58 PM Jessica Yu <[email protected]> wrote:
>>>>> +++ Masahiro Yamada [09/03/20 19:49 +0900]:
>>>>>> On Mon, Mar 9, 2020 at 7:39 PM Jessica Yu <[email protected]> wrote:
>>>>>>> Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing
>>>>>>> the 0-day bot emails are not CC'd to lkml. Here is the error I got
>>>>>>> from the bot:
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> I love your patch! Yet something to improve:
>>>>>>>
>>>>>>> [auto build test ERROR on linus/master]
>>>>>>> [also build test ERROR on v5.6-rc4 next-20200306]
>>>>>>> [if your patch is applied to the wrong git tree, please drop us a note to help
>>>>>>> improve the system. BTW, we also suggest to use '--base' option to specify the
>>>>>>> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>>>>>>>
>>>>>>> url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346
>>>>>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a
>>>>>>> config: sh-randconfig-a001-20200306 (attached as .config)
>>>>>>> compiler: sh4-linux-gcc (GCC) 7.5.0
>>>>>>> reproduce:
>>>>>>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>>>>> chmod +x ~/bin/make.cross
>>>>>>> # save the attached .config to linux build tree
>>>>>>> GCC_VERSION=7.5.0 make.cross ARCH=sh
>>>>>>>
>>>>>>> If you fix the issue, kindly add following tag
>>>>>>> Reported-by: kbuild test robot <[email protected]>
>>>>>>>
>>>>>>> All errors (new ones prefixed by >>):
>>>>>>>
>>>>>>>>> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>>>>>>
>>>>>> Indeed, this one is odd.
>>>>>> I have no idea...
>>>>> I've pushed the patches to a branch to let the kbuild bot run through its
>>>>> build tests again, and if I have extra time today I will try to
>>>>> reproduce this and let you know the results.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jessica
>>>>
>>>> Ah, Now I see.
>>>>
>>>>
>>>> Because you added "modpost:" prefix.
>>>>
>>>>
>>>>
>>>> The previous error message:
>>>>
>>>> ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>>>>
>>>>
>>>> The new error message:
>>>>
>>>> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>>>>
>>>>
>>>>
>>>> So, the bot assumed it was a new error.
>>> Nice catch! Hm, I suppose we need to let the LKP folks know about the
>>> change in error message. CC'd LKP mailing list.
>> Thanks for the info, we will look into this to handle the
>> changed error.
>>
>>> Jessica
>>> _______________________________________________
>>> LKP mailing list -- [email protected]
>>> To unsubscribe send an email to [email protected]
>
>
> Could you improve the report by adding more context?
>
> Currently, only new errors/warnings are shown by '>>'.
>
>
> If fixed ones had been shown by '<<',
> we would have easily noticed that
> this was just a matter of message format.
>
> << ERROR: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
>>> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!

Hi,

Thanks for your advice, we'll add it into our todo list.

Best Regards,
Rong Chen