Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1652241 > unrolled thread

linux-next: build failure after merge of the rcu tree

Started byStephen Rothwell <sfr@canb.auug.org.au>
First post2017-05-29 08:10 +0200
Last post2017-06-02 20:00 +0200
Articles 9 — 4 participants

Back to article view | Back to linux.kernel

Contents

  linux-next: build failure after merge of the rcu tree Stephen Rothwell <sfr@canb.auug.org.au> - 2017-05-29 08:10 +0200
    Re: linux-next: build failure after merge of the rcu tree "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-29 23:20 +0200
      Re: linux-next: build failure after merge of the rcu tree Stephen Rothwell <sfr@canb.auug.org.au> - 2017-05-30 03:50 +0200
        Re: linux-next: build failure after merge of the rcu tree Joe Perches <joe@perches.com> - 2017-05-30 04:00 +0200
          Re: linux-next: build failure after merge of the rcu tree "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-30 04:20 +0200
            Re: linux-next: build failure after merge of the rcu tree Joe Perches <joe@perches.com> - 2017-05-30 04:30 +0200
              Re: linux-next: build failure after merge of the rcu tree Stephen Rothwell <sfr@canb.auug.org.au> - 2017-05-30 05:20 +0200
      Re: linux-next: build failure after merge of the rcu tree Michael Ellerman <mpe@ellerman.id.au> - 2017-05-30 06:20 +0200
        Re: linux-next: build failure after merge of the rcu tree "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-02 20:00 +0200

#1652241 — linux-next: build failure after merge of the rcu tree

FromStephen Rothwell <sfr@canb.auug.org.au>
Date2017-05-29 08:10 +0200
Subjectlinux-next: build failure after merge of the rcu tree
Message-ID<tMm2l-4il-1@gated-at.bofh.it>
Hi Paul,

After merging the rcu tree, today's linux-next build (bfin
BF526-EZBRD_defconfig and several other bfin configs) failed like this:

In file included from include/linux/srcu.h:60:0,
                 from include/linux/notifier.h:15,
                 from include/linux/memory_hotplug.h:6,
                 from include/linux/mmzone.h:777,
                 from include/linux/gfp.h:5,
                 from include/linux/kmod.h:22,
                 from include/linux/module.h:13,
                 from include/linux/moduleloader.h:5,
                 from arch/blackfin/kernel/module.c:9:
include/linux/srcutiny.h: In function 'srcu_torture_stats_print':
include/linux/srcutiny.h:96:2: error: 'mod' undeclared (first use in this function)

Caused by commit

  54ffb22bd841 ("rcutorture: Move SRCU status printing to SRCU implementations")

arch/blackfin/kernel/module.c includes this:

#define pr_fmt(fmt) "module %s: " fmt, mod->name

added by commit

  b75a9e6bfba5 ("Blackfin: use more standard pr_fmt in the module loader")

before v2.6.39-rc2 ...

-- 
Cheers,
Stephen Rothwell

[toc] | [next] | [standalone]

#1652714

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2017-05-29 23:20 +0200
Message-ID<tMAf0-5J1-13@gated-at.bofh.it>
In reply to#1652241
On Mon, May 29, 2017 at 04:02:09PM +1000, Stephen Rothwell wrote:
> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build (bfin
> BF526-EZBRD_defconfig and several other bfin configs) failed like this:
> 
> In file included from include/linux/srcu.h:60:0,
>                  from include/linux/notifier.h:15,
>                  from include/linux/memory_hotplug.h:6,
>                  from include/linux/mmzone.h:777,
>                  from include/linux/gfp.h:5,
>                  from include/linux/kmod.h:22,
>                  from include/linux/module.h:13,
>                  from include/linux/moduleloader.h:5,
>                  from arch/blackfin/kernel/module.c:9:
> include/linux/srcutiny.h: In function 'srcu_torture_stats_print':
> include/linux/srcutiny.h:96:2: error: 'mod' undeclared (first use in this function)
> 
> Caused by commit
> 
>   54ffb22bd841 ("rcutorture: Move SRCU status printing to SRCU implementations")

And of course this is nothing but a printk().

> arch/blackfin/kernel/module.c includes this:
> 
> #define pr_fmt(fmt) "module %s: " fmt, mod->name

Hmmm...  There are well over 1000 definitions of pr_fmt() in the kernel.
Very popular name there!

> added by commit
> 
>   b75a9e6bfba5 ("Blackfin: use more standard pr_fmt in the module loader")
> 
> before v2.6.39-rc2 ...

I guess I cannot claim to be blindsided by a recent change, at least not
unless you take my age into account.  ;-)

So maybe there is some header file that blackfin wants included?  There
are only about 100 definitions of "mod", so let's see...  Well, none of
them are in blackfin, but perhaps the two in module.h?  But these are just
fields in structs.  As is the one in moduleparam.h.

At this point, the only option I see is to convert srcu_torture_stats_print()
from a static inline function to a macro.

Anyone see any other options?

								Thanx, Paul

[toc] | [prev] | [next] | [standalone]

#1652765

FromStephen Rothwell <sfr@canb.auug.org.au>
Date2017-05-30 03:50 +0200
Message-ID<tMEsi-cb-1@gated-at.bofh.it>
In reply to#1652714
Hi Paul,

On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>
> Anyone see any other options?

I will add the following to linux-next today.  If the Blackfin guys
agree (or come up with something better), you should get their acks and
then add it to your tree.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 30 May 2017 09:21:05 +1000
Subject: [PATCH] Revert "Blackfin: use more standard pr_fmt in the module loader"

This reverts commit b75a9e6bfba5c7343391ac1bacd44d6652dde8da.

It turns out (after all this time) that referencing a function parameter
from a global macro was a bit fragile.

This could probably be done better, but this fixes a build failure due
to a commit that adds a use of pr_alert in an include file.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/blackfin/kernel/module.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/arch/blackfin/kernel/module.c b/arch/blackfin/kernel/module.c
index 0188c933b155..adbbccc91fa1 100644
--- a/arch/blackfin/kernel/module.c
+++ b/arch/blackfin/kernel/module.c
@@ -4,7 +4,7 @@
  * Licensed under the GPL-2 or later
  */
 
-#define pr_fmt(fmt) "module %s: " fmt, mod->name
+#define pr_fmt(fmt) "module %s: " fmt
 
 #include <linux/moduleloader.h>
 #include <linux/elf.h>
@@ -44,7 +44,8 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_inst_sram_alloc(s->sh_size);
 			mod->arch.text_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 inst memory allocation failed\n");
+				pr_err("L1 inst memory allocation failed\n",
+					mod->name);
 				return -1;
 			}
 			dma_memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -56,7 +57,8 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_sram_alloc(s->sh_size);
 			mod->arch.data_a_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				pr_err("L1 data memory allocation failed\n",
+					mod->name);
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -68,7 +70,8 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_sram_zalloc(s->sh_size);
 			mod->arch.bss_a_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				pr_err("L1 data memory allocation failed\n",
+					mod->name);
 				return -1;
 			}
 
@@ -77,7 +80,8 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_B_sram_alloc(s->sh_size);
 			mod->arch.data_b_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				pr_err("L1 data memory allocation failed\n",
+					mod->name);
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -87,7 +91,8 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_B_sram_alloc(s->sh_size);
 			mod->arch.bss_b_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				pr_err("L1 data memory allocation failed\n",
+					mod->name);
 				return -1;
 			}
 			memset(dest, 0, s->sh_size);
@@ -99,7 +104,8 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_alloc(s->sh_size);
 			mod->arch.text_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				pr_err("L2 SRAM allocation failed\n",
+					mod->name);
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -111,7 +117,8 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_alloc(s->sh_size);
 			mod->arch.data_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				pr_err("L2 SRAM allocation failed\n",
+					mod->name);
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -123,7 +130,8 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_zalloc(s->sh_size);
 			mod->arch.bss_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				pr_err("L2 SRAM allocation failed\n",
+					mod->name);
 				return -1;
 			}
 
@@ -157,7 +165,7 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 	Elf32_Sym *sym;
 	unsigned long location, value, size;
 
-	pr_debug("applying relocate section %u to %u\n",
+	pr_debug("applying relocate section %u to %u\n", mod->name,
 		relsec, sechdrs[relsec].sh_info);
 
 	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
@@ -174,14 +182,14 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 
 #ifdef CONFIG_SMP
 		if (location >= COREB_L1_DATA_A_START) {
-			pr_err("cannot relocate in L1: %u (SMP kernel)\n",
-				ELF32_R_TYPE(rel[i].r_info));
+			pr_err("cannot relocate in L1: %u (SMP kernel)",
+				mod->name, ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 		}
 #endif
 
 		pr_debug("location is %lx, value is %lx type is %d\n",
-			location, value, ELF32_R_TYPE(rel[i].r_info));
+			mod->name, location, value, ELF32_R_TYPE(rel[i].r_info));
 
 		switch (ELF32_R_TYPE(rel[i].r_info)) {
 
@@ -201,11 +209,11 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 		case R_BFIN_PCREL12_JUMP_S:
 		case R_BFIN_PCREL10:
 			pr_err("unsupported relocation: %u (no -mlong-calls?)\n",
-				ELF32_R_TYPE(rel[i].r_info));
+				mod->name, ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 
 		default:
-			pr_err("unknown relocation: %u\n",
+			pr_err("unknown relocation: %u\n", mod->name,
 				ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 		}
@@ -222,7 +230,8 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 			isram_memcpy((void *)location, &value, size);
 			break;
 		default:
-			pr_err("invalid relocation for %#lx\n", location);
+			pr_err("invalid relocation for %#lx\n",
+				mod->name, location);
 			return -ENOEXEC;
 		}
 	}
-- 
2.11.0

-- 
Cheers,
Stephen Rothwell

[toc] | [prev] | [next] | [standalone]

#1652766

FromJoe Perches <joe@perches.com>
Date2017-05-30 04:00 +0200
Message-ID<tMEBX-go-3@gated-at.bofh.it>
In reply to#1652765
On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:
> Hi Paul,
> 
> > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > Anyone see any other options?

My preferred option would be removing pr_fmt
and adding a couple new macros.
---
 arch/blackfin/kernel/module.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/arch/blackfin/kernel/module.c b/arch/blackfin/kernel/module.c
index 0188c933b155..e43aec7eb8d3 100644
--- a/arch/blackfin/kernel/module.c
+++ b/arch/blackfin/kernel/module.c
@@ -4,8 +4,6 @@
  * Licensed under the GPL-2 or later
  */
 
-#define pr_fmt(fmt) "module %s: " fmt, mod->name
-
 #include <linux/moduleloader.h>
 #include <linux/elf.h>
 #include <linux/vmalloc.h>
@@ -16,6 +14,11 @@
 #include <asm/cacheflush.h>
 #include <linux/uaccess.h>
 
+#define mod_err(mod, fmt, ...)						\
+	pr_err("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
+#define mod_debug(mod, ...)						\
+	pr_debug("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
+
 /* Transfer the section to the L1 memory */
 int
 module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
@@ -44,7 +47,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_inst_sram_alloc(s->sh_size);
 			mod->arch.text_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 inst memory allocation failed\n");
+				mod_err(mod, "L1 inst memory allocation failed\n");
 				return -1;
 			}
 			dma_memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -56,7 +59,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_sram_alloc(s->sh_size);
 			mod->arch.data_a_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				mod_err(mod, "L1 data memory allocation failed\n");
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -68,7 +71,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_sram_zalloc(s->sh_size);
 			mod->arch.bss_a_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				mod_err(mod, "L1 data memory allocation failed\n");
 				return -1;
 			}
 
@@ -77,7 +80,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_B_sram_alloc(s->sh_size);
 			mod->arch.data_b_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				mod_err(mod, "L1 data memory allocation failed\n");
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -87,7 +90,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_B_sram_alloc(s->sh_size);
 			mod->arch.bss_b_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				mod_err(mod, "L1 data memory allocation failed\n");
 				return -1;
 			}
 			memset(dest, 0, s->sh_size);
@@ -99,7 +102,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_alloc(s->sh_size);
 			mod->arch.text_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				mod_err(mod, "L2 SRAM allocation failed\n");
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -111,7 +114,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_alloc(s->sh_size);
 			mod->arch.data_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				mod_err(mod, "L2 SRAM allocation failed\n");
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -123,7 +126,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_zalloc(s->sh_size);
 			mod->arch.bss_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				mod_err(mod, "L2 SRAM allocation failed\n");
 				return -1;
 			}
 
@@ -157,8 +160,8 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 	Elf32_Sym *sym;
 	unsigned long location, value, size;
 
-	pr_debug("applying relocate section %u to %u\n",
-		relsec, sechdrs[relsec].sh_info);
+	mod_debug(mod, "applying relocate section %u to %u\n",
+		  relsec, sechdrs[relsec].sh_info);
 
 	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
 		/* This is where to make the change */
@@ -174,14 +177,14 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 
 #ifdef CONFIG_SMP
 		if (location >= COREB_L1_DATA_A_START) {
-			pr_err("cannot relocate in L1: %u (SMP kernel)\n",
+			mod_err(mod, "cannot relocate in L1: %u (SMP kernel)\n",
 				ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 		}
 #endif
 
-		pr_debug("location is %lx, value is %lx type is %d\n",
-			location, value, ELF32_R_TYPE(rel[i].r_info));
+		mod_debug(mod, "location is %lx, value is %lx type is %d\n",
+			  location, value, ELF32_R_TYPE(rel[i].r_info));
 
 		switch (ELF32_R_TYPE(rel[i].r_info)) {
 
@@ -200,12 +203,12 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 		case R_BFIN_PCREL12_JUMP:
 		case R_BFIN_PCREL12_JUMP_S:
 		case R_BFIN_PCREL10:
-			pr_err("unsupported relocation: %u (no -mlong-calls?)\n",
+			mod_err(mod, "unsupported relocation: %u (no -mlong-calls?)\n",
 				ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 
 		default:
-			pr_err("unknown relocation: %u\n",
+			mod_err(mod, "unknown relocation: %u\n",
 				ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 		}
@@ -222,7 +225,7 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 			isram_memcpy((void *)location, &value, size);
 			break;
 		default:
-			pr_err("invalid relocation for %#lx\n", location);
+			mod_err(mod, "invalid relocation for %#lx\n", location);
 			return -ENOEXEC;
 		}
 	}

[toc] | [prev] | [next] | [standalone]

#1652769

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2017-05-30 04:20 +0200
Message-ID<tMEVj-LL-5@gated-at.bofh.it>
In reply to#1652766
On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:
> On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:
> > Hi Paul,
> > 
> > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > > 
> > > Anyone see any other options?
> 
> My preferred option would be removing pr_fmt
> and adding a couple new macros.

Not sure how to evaluate yours and Stephen's changes, but I reverted my
conversion to a macro based on the hope that something good will come
of this effort.  ;-)

							Thanx, Paul

> ---
>  arch/blackfin/kernel/module.c | 39 +++++++++++++++++++++------------------
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/blackfin/kernel/module.c b/arch/blackfin/kernel/module.c
> index 0188c933b155..e43aec7eb8d3 100644
> --- a/arch/blackfin/kernel/module.c
> +++ b/arch/blackfin/kernel/module.c
> @@ -4,8 +4,6 @@
>   * Licensed under the GPL-2 or later
>   */
>  
> -#define pr_fmt(fmt) "module %s: " fmt, mod->name
> -
>  #include <linux/moduleloader.h>
>  #include <linux/elf.h>
>  #include <linux/vmalloc.h>
> @@ -16,6 +14,11 @@
>  #include <asm/cacheflush.h>
>  #include <linux/uaccess.h>
>  
> +#define mod_err(mod, fmt, ...)						\
> +	pr_err("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
> +#define mod_debug(mod, ...)						\
> +	pr_debug("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
> +
>  /* Transfer the section to the L1 memory */
>  int
>  module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> @@ -44,7 +47,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  			dest = l1_inst_sram_alloc(s->sh_size);
>  			mod->arch.text_l1 = dest;
>  			if (dest == NULL) {
> -				pr_err("L1 inst memory allocation failed\n");
> +				mod_err(mod, "L1 inst memory allocation failed\n");
>  				return -1;
>  			}
>  			dma_memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -56,7 +59,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  			dest = l1_data_sram_alloc(s->sh_size);
>  			mod->arch.data_a_l1 = dest;
>  			if (dest == NULL) {
> -				pr_err("L1 data memory allocation failed\n");
> +				mod_err(mod, "L1 data memory allocation failed\n");
>  				return -1;
>  			}
>  			memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -68,7 +71,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  			dest = l1_data_sram_zalloc(s->sh_size);
>  			mod->arch.bss_a_l1 = dest;
>  			if (dest == NULL) {
> -				pr_err("L1 data memory allocation failed\n");
> +				mod_err(mod, "L1 data memory allocation failed\n");
>  				return -1;
>  			}
>  
> @@ -77,7 +80,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  			dest = l1_data_B_sram_alloc(s->sh_size);
>  			mod->arch.data_b_l1 = dest;
>  			if (dest == NULL) {
> -				pr_err("L1 data memory allocation failed\n");
> +				mod_err(mod, "L1 data memory allocation failed\n");
>  				return -1;
>  			}
>  			memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -87,7 +90,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  			dest = l1_data_B_sram_alloc(s->sh_size);
>  			mod->arch.bss_b_l1 = dest;
>  			if (dest == NULL) {
> -				pr_err("L1 data memory allocation failed\n");
> +				mod_err(mod, "L1 data memory allocation failed\n");
>  				return -1;
>  			}
>  			memset(dest, 0, s->sh_size);
> @@ -99,7 +102,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  			dest = l2_sram_alloc(s->sh_size);
>  			mod->arch.text_l2 = dest;
>  			if (dest == NULL) {
> -				pr_err("L2 SRAM allocation failed\n");
> +				mod_err(mod, "L2 SRAM allocation failed\n");
>  				return -1;
>  			}
>  			memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -111,7 +114,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  			dest = l2_sram_alloc(s->sh_size);
>  			mod->arch.data_l2 = dest;
>  			if (dest == NULL) {
> -				pr_err("L2 SRAM allocation failed\n");
> +				mod_err(mod, "L2 SRAM allocation failed\n");
>  				return -1;
>  			}
>  			memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -123,7 +126,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  			dest = l2_sram_zalloc(s->sh_size);
>  			mod->arch.bss_l2 = dest;
>  			if (dest == NULL) {
> -				pr_err("L2 SRAM allocation failed\n");
> +				mod_err(mod, "L2 SRAM allocation failed\n");
>  				return -1;
>  			}
>  
> @@ -157,8 +160,8 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>  	Elf32_Sym *sym;
>  	unsigned long location, value, size;
>  
> -	pr_debug("applying relocate section %u to %u\n",
> -		relsec, sechdrs[relsec].sh_info);
> +	mod_debug(mod, "applying relocate section %u to %u\n",
> +		  relsec, sechdrs[relsec].sh_info);
>  
>  	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
>  		/* This is where to make the change */
> @@ -174,14 +177,14 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>  
>  #ifdef CONFIG_SMP
>  		if (location >= COREB_L1_DATA_A_START) {
> -			pr_err("cannot relocate in L1: %u (SMP kernel)\n",
> +			mod_err(mod, "cannot relocate in L1: %u (SMP kernel)\n",
>  				ELF32_R_TYPE(rel[i].r_info));
>  			return -ENOEXEC;
>  		}
>  #endif
>  
> -		pr_debug("location is %lx, value is %lx type is %d\n",
> -			location, value, ELF32_R_TYPE(rel[i].r_info));
> +		mod_debug(mod, "location is %lx, value is %lx type is %d\n",
> +			  location, value, ELF32_R_TYPE(rel[i].r_info));
>  
>  		switch (ELF32_R_TYPE(rel[i].r_info)) {
>  
> @@ -200,12 +203,12 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>  		case R_BFIN_PCREL12_JUMP:
>  		case R_BFIN_PCREL12_JUMP_S:
>  		case R_BFIN_PCREL10:
> -			pr_err("unsupported relocation: %u (no -mlong-calls?)\n",
> +			mod_err(mod, "unsupported relocation: %u (no -mlong-calls?)\n",
>  				ELF32_R_TYPE(rel[i].r_info));
>  			return -ENOEXEC;
>  
>  		default:
> -			pr_err("unknown relocation: %u\n",
> +			mod_err(mod, "unknown relocation: %u\n",
>  				ELF32_R_TYPE(rel[i].r_info));
>  			return -ENOEXEC;
>  		}
> @@ -222,7 +225,7 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>  			isram_memcpy((void *)location, &value, size);
>  			break;
>  		default:
> -			pr_err("invalid relocation for %#lx\n", location);
> +			mod_err(mod, "invalid relocation for %#lx\n", location);
>  			return -ENOEXEC;
>  		}
>  	}
>

[toc] | [prev] | [next] | [standalone]

#1652776

FromJoe Perches <joe@perches.com>
Date2017-05-30 04:30 +0200
Message-ID<tMF4Z-PL-3@gated-at.bofh.it>
In reply to#1652769
On Mon, 2017-05-29 at 19:14 -0700, Paul E. McKenney wrote:
> On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:
> > On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:
> > > Hi Paul,
> > > 
> > > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > > > 
> > > > Anyone see any other options?
> > 
> > My preferred option would be removing pr_fmt
> > and adding a couple new macros.
> 
> Not sure how to evaluate yours and Stephen's changes, but I reverted my
> conversion to a macro based on the hope that something good will come
> of this effort.  ;-)

Stephen's suggestion makes the format and arguments
have an apparent mismatch.  What I suggested hides
the "module %s: ", mod->name bit in the macros (like
the older pr_fmt use), allows anyone else to #define
pr_fmt to taste, and keeps the format and arguments in
agreement.

[toc] | [prev] | [next] | [standalone]

#1652788

FromStephen Rothwell <sfr@canb.auug.org.au>
Date2017-05-30 05:20 +0200
Message-ID<tMFRn-1q5-3@gated-at.bofh.it>
In reply to#1652776
Hi Joe,

On Mon, 29 May 2017 19:20:25 -0700 Joe Perches <joe@perches.com> wrote:
>
> On Mon, 2017-05-29 at 19:14 -0700, Paul E. McKenney wrote:
> > On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:  
> > > On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:  
> > > > Hi Paul,
> > > >   
> > > > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > > > > 
> > > > > Anyone see any other options?  
> > > 
> > > My preferred option would be removing pr_fmt
> > > and adding a couple new macros.  
> > 
> > Not sure how to evaluate yours and Stephen's changes, but I reverted my
> > conversion to a macro based on the hope that something good will come
> > of this effort.  ;-)  
> 
> Stephen's suggestion makes the format and arguments
> have an apparent mismatch.  What I suggested hides
> the "module %s: ", mod->name bit in the macros (like
> the older pr_fmt use), allows anyone else to #define
> pr_fmt to taste, and keeps the format and arguments in
> agreement.

Yours is much better, mine was just a quick hack ... consider yours

Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>

-- 
Cheers,
Stephen Rothwell

[toc] | [prev] | [next] | [standalone]

#1652800

FromMichael Ellerman <mpe@ellerman.id.au>
Date2017-05-30 06:20 +0200
Message-ID<tMGNr-25z-7@gated-at.bofh.it>
In reply to#1652714
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> writes:

> On Mon, May 29, 2017 at 04:02:09PM +1000, Stephen Rothwell wrote:
>> Hi Paul,
>> 
>> After merging the rcu tree, today's linux-next build (bfin
>> BF526-EZBRD_defconfig and several other bfin configs) failed like this:
>> 
>> In file included from include/linux/srcu.h:60:0,
>>                  from include/linux/notifier.h:15,
>>                  from include/linux/memory_hotplug.h:6,
>>                  from include/linux/mmzone.h:777,
>>                  from include/linux/gfp.h:5,
>>                  from include/linux/kmod.h:22,
>>                  from include/linux/module.h:13,
>>                  from include/linux/moduleloader.h:5,
>>                  from arch/blackfin/kernel/module.c:9:
>> include/linux/srcutiny.h: In function 'srcu_torture_stats_print':
>> include/linux/srcutiny.h:96:2: error: 'mod' undeclared (first use in this function)
>> 
>> Caused by commit
>> 
>>   54ffb22bd841 ("rcutorture: Move SRCU status printing to SRCU implementations")
>
> And of course this is nothing but a printk().

Ah but it's not, it's a pr_alert():

+       pr_alert("%s%s Tiny SRCU per-CPU(idx=%d): (%hd,%hd)\n",
+                tt, tf, idx,
+                READ_ONCE(sp->srcu_lock_nesting[!idx]),
+                READ_ONCE(sp->srcu_lock_nesting[idx]));

Where pr_alert() is:

#define pr_alert(fmt, ...) \
	printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)


So that's where pr_fmt() is coming into it.

And if any code in module.c called srcu_torture_stats_print(), it would
actually claim to come from "module: ", which would be confusing.

I don't think you use pr_fmt() in the RCU code, so you could skip using
pr_alert() and just use printk(KERN_ALERT "...") and get the same
result, without any interactions with pr_fmt().

cheers

[toc] | [prev] | [next] | [standalone]

#1656429

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2017-06-02 20:00 +0200
Message-ID<tNZ1E-4jG-15@gated-at.bofh.it>
In reply to#1652800
On Tue, May 30, 2017 at 02:10:26PM +1000, Michael Ellerman wrote:
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> writes:
> 
> > On Mon, May 29, 2017 at 04:02:09PM +1000, Stephen Rothwell wrote:
> >> Hi Paul,
> >> 
> >> After merging the rcu tree, today's linux-next build (bfin
> >> BF526-EZBRD_defconfig and several other bfin configs) failed like this:
> >> 
> >> In file included from include/linux/srcu.h:60:0,
> >>                  from include/linux/notifier.h:15,
> >>                  from include/linux/memory_hotplug.h:6,
> >>                  from include/linux/mmzone.h:777,
> >>                  from include/linux/gfp.h:5,
> >>                  from include/linux/kmod.h:22,
> >>                  from include/linux/module.h:13,
> >>                  from include/linux/moduleloader.h:5,
> >>                  from arch/blackfin/kernel/module.c:9:
> >> include/linux/srcutiny.h: In function 'srcu_torture_stats_print':
> >> include/linux/srcutiny.h:96:2: error: 'mod' undeclared (first use in this function)
> >> 
> >> Caused by commit
> >> 
> >>   54ffb22bd841 ("rcutorture: Move SRCU status printing to SRCU implementations")
> >
> > And of course this is nothing but a printk().
> 
> Ah but it's not, it's a pr_alert():
> 
> +       pr_alert("%s%s Tiny SRCU per-CPU(idx=%d): (%hd,%hd)\n",
> +                tt, tf, idx,
> +                READ_ONCE(sp->srcu_lock_nesting[!idx]),
> +                READ_ONCE(sp->srcu_lock_nesting[idx]));
> 
> Where pr_alert() is:
> 
> #define pr_alert(fmt, ...) \
> 	printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> 
> 
> So that's where pr_fmt() is coming into it.
> 
> And if any code in module.c called srcu_torture_stats_print(), it would
> actually claim to come from "module: ", which would be confusing.
> 
> I don't think you use pr_fmt() in the RCU code, so you could skip using
> pr_alert() and just use printk(KERN_ALERT "...") and get the same
> result, without any interactions with pr_fmt().

Does the following patch fix things for you?

							Thanx, Paul

------------------------------------------------------------------------

commit 7eb8d998d3f66851c0d25abcdb2d737edc6a21ed
Author: Joe Perches <joe@perches.com>
Date:   Tue May 30 15:20:37 2017 -0700

    module: Fix pr_fmt() bug for header use of printk
    
    This commit removes the pr_fmt() macro, replacing it with mod_err() and
    mod_debug() macros to avoid errors when using printk() from header files.
    
    Signed-off-by: Joe Perches <joe@perches.com>
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/arch/blackfin/kernel/module.c b/arch/blackfin/kernel/module.c
index 0188c933b155..15af5768c403 100644
--- a/arch/blackfin/kernel/module.c
+++ b/arch/blackfin/kernel/module.c
@@ -4,8 +4,6 @@
  * Licensed under the GPL-2 or later
  */
 
-#define pr_fmt(fmt) "module %s: " fmt, mod->name
-
 #include <linux/moduleloader.h>
 #include <linux/elf.h>
 #include <linux/vmalloc.h>
@@ -16,6 +14,11 @@
 #include <asm/cacheflush.h>
 #include <linux/uaccess.h>
 
+#define mod_err(mod, fmt, ...)						\
+	pr_err("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
+#define mod_debug(mod, fmt, ...)					\
+	pr_debug("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
+
 /* Transfer the section to the L1 memory */
 int
 module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
@@ -44,7 +47,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_inst_sram_alloc(s->sh_size);
 			mod->arch.text_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 inst memory allocation failed\n");
+				mod_err(mod, "L1 inst memory allocation failed\n");
 				return -1;
 			}
 			dma_memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -56,7 +59,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_sram_alloc(s->sh_size);
 			mod->arch.data_a_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				mod_err(mod, "L1 data memory allocation failed\n");
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -68,7 +71,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_sram_zalloc(s->sh_size);
 			mod->arch.bss_a_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				mod_err(mod, "L1 data memory allocation failed\n");
 				return -1;
 			}
 
@@ -77,7 +80,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_B_sram_alloc(s->sh_size);
 			mod->arch.data_b_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				mod_err(mod, "L1 data memory allocation failed\n");
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -87,7 +90,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l1_data_B_sram_alloc(s->sh_size);
 			mod->arch.bss_b_l1 = dest;
 			if (dest == NULL) {
-				pr_err("L1 data memory allocation failed\n");
+				mod_err(mod, "L1 data memory allocation failed\n");
 				return -1;
 			}
 			memset(dest, 0, s->sh_size);
@@ -99,7 +102,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_alloc(s->sh_size);
 			mod->arch.text_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				mod_err(mod, "L2 SRAM allocation failed\n");
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -111,7 +114,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_alloc(s->sh_size);
 			mod->arch.data_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				mod_err(mod, "L2 SRAM allocation failed\n");
 				return -1;
 			}
 			memcpy(dest, (void *)s->sh_addr, s->sh_size);
@@ -123,7 +126,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 			dest = l2_sram_zalloc(s->sh_size);
 			mod->arch.bss_l2 = dest;
 			if (dest == NULL) {
-				pr_err("L2 SRAM allocation failed\n");
+				mod_err(mod, "L2 SRAM allocation failed\n");
 				return -1;
 			}
 
@@ -157,8 +160,8 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 	Elf32_Sym *sym;
 	unsigned long location, value, size;
 
-	pr_debug("applying relocate section %u to %u\n",
-		relsec, sechdrs[relsec].sh_info);
+	mod_debug(mod, "applying relocate section %u to %u\n",
+		  relsec, sechdrs[relsec].sh_info);
 
 	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
 		/* This is where to make the change */
@@ -174,14 +177,14 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 
 #ifdef CONFIG_SMP
 		if (location >= COREB_L1_DATA_A_START) {
-			pr_err("cannot relocate in L1: %u (SMP kernel)\n",
+			mod_err(mod, "cannot relocate in L1: %u (SMP kernel)\n",
 				ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 		}
 #endif
 
-		pr_debug("location is %lx, value is %lx type is %d\n",
-			location, value, ELF32_R_TYPE(rel[i].r_info));
+		mod_debug(mod, "location is %lx, value is %lx type is %d\n",
+			  location, value, ELF32_R_TYPE(rel[i].r_info));
 
 		switch (ELF32_R_TYPE(rel[i].r_info)) {
 
@@ -200,12 +203,12 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 		case R_BFIN_PCREL12_JUMP:
 		case R_BFIN_PCREL12_JUMP_S:
 		case R_BFIN_PCREL10:
-			pr_err("unsupported relocation: %u (no -mlong-calls?)\n",
+			mod_err(mod, "unsupported relocation: %u (no -mlong-calls?)\n",
 				ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 
 		default:
-			pr_err("unknown relocation: %u\n",
+			mod_err(mod, "unknown relocation: %u\n",
 				ELF32_R_TYPE(rel[i].r_info));
 			return -ENOEXEC;
 		}
@@ -222,7 +225,7 @@ apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 			isram_memcpy((void *)location, &value, size);
 			break;
 		default:
-			pr_err("invalid relocation for %#lx\n", location);
+			mod_err(mod, "invalid relocation for %#lx\n", location);
 			return -ENOEXEC;
 		}
 	}

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web