[mips-next,1/2] MIPS: module: optimize module relocations processing
diff mbox series

Message ID 20210116150225.21127-1-alobakin@pm.me
State Accepted
Commit 049a68efbf0bb29d012f2d73b91c674ca2d805fe
Headers show
Series
  • MIPS: optimize relocations processing
Related show

Commit Message

Alexander Lobakin Jan. 16, 2021, 3:02 p.m. UTC
For now, module relocation functions are implemented as an array
of handlers of type reloc_handler_t.

Convert that array into a single switch-case function to:
 - remove unused arguments;
 - change the return type of simple handlers to void;
 - remove the array and don't use any data at all;
 - avoid using indirect calls;
 - allow the compiler to inline and greatly optimize
   the relocation function[s].

The result on MIPS32 R2 with GCC 10.2 -O2 is:

scripts/bloat-o-meter -c arch/mips/kernel/__module.o arch/mips/kernel/module.o
add/remove: 1/11 grow/shrink: 1/0 up/down: 876/-1436 (-560)
Function                                     old     new   delta
apply_relocate                               456    1148    +692
apply_r_mips_pc                                -     184    +184
apply_r_mips_none                              8       -      -8
apply_r_mips_32                               16       -     -16
apply_r_mips_64                               76       -     -76
apply_r_mips_highest                          88       -     -88
apply_r_mips_higher                          108       -    -108
apply_r_mips_26                              132       -    -132
apply_r_mips_pc26                            160       -    -160
apply_r_mips_pc21                            160       -    -160
apply_r_mips_pc16                            160       -    -160
apply_r_mips_hi16                            172       -    -172
apply_r_mips_lo16                            356       -    -356
Total: Before=2608, After=2048, chg -21.47%
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
Data                                         old     new   delta
Total: Before=12, After=12, chg +0.00%
add/remove: 0/1 grow/shrink: 0/0 up/down: 0/-248 (-248)
RO Data                                      old     new   delta
reloc_handlers                               248       -    -248
Total: Before=248, After=0, chg -100.00%

All functions were collapsed into a single one that is called
directly by $(srctree)/kernel/module.c.

Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 arch/mips/kernel/module.c | 109 ++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 57 deletions(-)

Comments

Jiaxun Yang Jan. 18, 2021, 1:49 a.m. UTC | #1
在 2021/1/16 下午11:02, Alexander Lobakin 写道:
> For now, module relocation functions are implemented as an array
> of handlers of type reloc_handler_t.
>
> Convert that array into a single switch-case function to:
>   - remove unused arguments;
>   - change the return type of simple handlers to void;
>   - remove the array and don't use any data at all;
>   - avoid using indirect calls;
>   - allow the compiler to inline and greatly optimize
>     the relocation function[s].
>
> The result on MIPS32 R2 with GCC 10.2 -O2 is:
>
> scripts/bloat-o-meter -c arch/mips/kernel/__module.o arch/mips/kernel/module.o
> add/remove: 1/11 grow/shrink: 1/0 up/down: 876/-1436 (-560)
> Function                                     old     new   delta
> apply_relocate                               456    1148    +692
> apply_r_mips_pc                                -     184    +184
> apply_r_mips_none                              8       -      -8
> apply_r_mips_32                               16       -     -16
> apply_r_mips_64                               76       -     -76
> apply_r_mips_highest                          88       -     -88
> apply_r_mips_higher                          108       -    -108
> apply_r_mips_26                              132       -    -132
> apply_r_mips_pc26                            160       -    -160
> apply_r_mips_pc21                            160       -    -160
> apply_r_mips_pc16                            160       -    -160
> apply_r_mips_hi16                            172       -    -172
> apply_r_mips_lo16                            356       -    -356
> Total: Before=2608, After=2048, chg -21.47%
> add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> Data                                         old     new   delta
> Total: Before=12, After=12, chg +0.00%
> add/remove: 0/1 grow/shrink: 0/0 up/down: 0/-248 (-248)
> RO Data                                      old     new   delta
> reloc_handlers                               248       -    -248
> Total: Before=248, After=0, chg -100.00%
>
> All functions were collapsed into a single one that is called
> directly by $(srctree)/kernel/module.c.
>
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>

> ---
>   arch/mips/kernel/module.c | 109 ++++++++++++++++++--------------------
>   1 file changed, 52 insertions(+), 57 deletions(-)
>
> diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
> index 3c0c3d1260c1..14f46d17500a 100644
> --- a/arch/mips/kernel/module.c
> +++ b/arch/mips/kernel/module.c
> @@ -40,22 +40,13 @@ void *module_alloc(unsigned long size)
>   }
>   #endif
>   
> -static int apply_r_mips_none(struct module *me, u32 *location,
> -			     u32 base, Elf_Addr v, bool rela)
> -{
> -	return 0;
> -}
> -
> -static int apply_r_mips_32(struct module *me, u32 *location,
> -			   u32 base, Elf_Addr v, bool rela)
> +static void apply_r_mips_32(u32 *location, u32 base, Elf_Addr v)
>   {
>   	*location = base + v;
> -
> -	return 0;
>   }
>   
> -static int apply_r_mips_26(struct module *me, u32 *location,
> -			   u32 base, Elf_Addr v, bool rela)
> +static int apply_r_mips_26(struct module *me, u32 *location, u32 base,
> +			   Elf_Addr v)
>   {
>   	if (v % 4) {
>   		pr_err("module %s: dangerous R_MIPS_26 relocation\n",
> @@ -75,8 +66,8 @@ static int apply_r_mips_26(struct module *me, u32 *location,
>   	return 0;
>   }
>   
> -static int apply_r_mips_hi16(struct module *me, u32 *location,
> -			     u32 base, Elf_Addr v, bool rela)
> +static int apply_r_mips_hi16(struct module *me, u32 *location, Elf_Addr v,
> +			     bool rela)
>   {
>   	struct mips_hi16 *n;
>   
> @@ -217,26 +208,25 @@ static int apply_r_mips_pc(struct module *me, u32 *location, u32 base,
>   	return 0;
>   }
>   
> -static int apply_r_mips_pc16(struct module *me, u32 *location,
> -			     u32 base, Elf_Addr v, bool rela)
> +static int apply_r_mips_pc16(struct module *me, u32 *location, u32 base,
> +			     Elf_Addr v)
>   {
>   	return apply_r_mips_pc(me, location, base, v, 16);
>   }
>   
> -static int apply_r_mips_pc21(struct module *me, u32 *location,
> -			     u32 base, Elf_Addr v, bool rela)
> +static int apply_r_mips_pc21(struct module *me, u32 *location, u32 base,
> +			     Elf_Addr v)
>   {
>   	return apply_r_mips_pc(me, location, base, v, 21);
>   }
>   
> -static int apply_r_mips_pc26(struct module *me, u32 *location,
> -			     u32 base, Elf_Addr v, bool rela)
> +static int apply_r_mips_pc26(struct module *me, u32 *location, u32 base,
> +			     Elf_Addr v)
>   {
>   	return apply_r_mips_pc(me, location, base, v, 26);
>   }
>   
> -static int apply_r_mips_64(struct module *me, u32 *location,
> -			   u32 base, Elf_Addr v, bool rela)
> +static int apply_r_mips_64(u32 *location, Elf_Addr v, bool rela)
>   {
>   	if (WARN_ON(!rela))
>   		return -EINVAL;
> @@ -246,8 +236,7 @@ static int apply_r_mips_64(struct module *me, u32 *location,
>   	return 0;
>   }
>   
> -static int apply_r_mips_higher(struct module *me, u32 *location,
> -			       u32 base, Elf_Addr v, bool rela)
> +static int apply_r_mips_higher(u32 *location, Elf_Addr v, bool rela)
>   {
>   	if (WARN_ON(!rela))
>   		return -EINVAL;
> @@ -258,8 +247,7 @@ static int apply_r_mips_higher(struct module *me, u32 *location,
>   	return 0;
>   }
>   
> -static int apply_r_mips_highest(struct module *me, u32 *location,
> -				u32 base, Elf_Addr v, bool rela)
> +static int apply_r_mips_highest(u32 *location, Elf_Addr v, bool rela)
>   {
>   	if (WARN_ON(!rela))
>   		return -EINVAL;
> @@ -272,12 +260,14 @@ static int apply_r_mips_highest(struct module *me, u32 *location,
>   
>   /**
>    * reloc_handler() - Apply a particular relocation to a module
> + * @type: type of the relocation to apply
>    * @me: the module to apply the reloc to
>    * @location: the address at which the reloc is to be applied
>    * @base: the existing value at location for REL-style; 0 for RELA-style
>    * @v: the value of the reloc, with addend for RELA-style
> + * @rela: indication of is this a RELA (true) or REL (false) relocation
>    *
> - * Each implemented reloc_handler function applies a particular type of
> + * Each implemented relocation function applies a particular type of
>    * relocation to the module @me. Relocs that may be found in either REL or RELA
>    * variants can be handled by making use of the @base & @v parameters which are
>    * set to values which abstract the difference away from the particular reloc
> @@ -285,23 +275,40 @@ static int apply_r_mips_highest(struct module *me, u32 *location,
>    *
>    * Return: 0 upon success, else -ERRNO
>    */
> -typedef int (*reloc_handler)(struct module *me, u32 *location,
> -			     u32 base, Elf_Addr v, bool rela);
> -
> -/* The handlers for known reloc types */
> -static reloc_handler reloc_handlers[] = {
> -	[R_MIPS_NONE]		= apply_r_mips_none,
> -	[R_MIPS_32]		= apply_r_mips_32,
> -	[R_MIPS_26]		= apply_r_mips_26,
> -	[R_MIPS_HI16]		= apply_r_mips_hi16,
> -	[R_MIPS_LO16]		= apply_r_mips_lo16,
> -	[R_MIPS_PC16]		= apply_r_mips_pc16,
> -	[R_MIPS_64]		= apply_r_mips_64,
> -	[R_MIPS_HIGHER]		= apply_r_mips_higher,
> -	[R_MIPS_HIGHEST]	= apply_r_mips_highest,
> -	[R_MIPS_PC21_S2]	= apply_r_mips_pc21,
> -	[R_MIPS_PC26_S2]	= apply_r_mips_pc26,
> -};
> +static int reloc_handler(u32 type, struct module *me, u32 *location, u32 base,
> +			 Elf_Addr v, bool rela)
> +{
> +	switch (type) {
> +	case R_MIPS_NONE:
> +		break;
> +	case R_MIPS_32:
> +		apply_r_mips_32(location, base, v);
> +		break;
> +	case R_MIPS_26:
> +		return apply_r_mips_26(me, location, base, v);
> +	case R_MIPS_HI16:
> +		return apply_r_mips_hi16(me, location, v, rela);
> +	case R_MIPS_LO16:
> +		return apply_r_mips_lo16(me, location, base, v, rela);
> +	case R_MIPS_PC16:
> +		return apply_r_mips_pc16(me, location, base, v);
> +	case R_MIPS_PC21_S2:
> +		return apply_r_mips_pc21(me, location, base, v);
> +	case R_MIPS_PC26_S2:
> +		return apply_r_mips_pc26(me, location, base, v);
> +	case R_MIPS_64:
> +		return apply_r_mips_64(location, v, rela);
> +	case R_MIPS_HIGHER:
> +		return apply_r_mips_higher(location, v, rela);
> +	case R_MIPS_HIGHEST:
> +		return apply_r_mips_highest(location, v, rela);
> +	default:
> +		pr_err("%s: Unknown relocation type %u\n", me->name, type);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
>   
>   static int __apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
>   			    unsigned int symindex, unsigned int relsec,
> @@ -311,7 +318,6 @@ static int __apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
>   		Elf_Mips_Rel *rel;
>   		Elf_Mips_Rela *rela;
>   	} r;
> -	reloc_handler handler;
>   	Elf_Sym *sym;
>   	u32 *location, base;
>   	unsigned int i, type;
> @@ -343,17 +349,6 @@ static int __apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
>   		}
>   
>   		type = ELF_MIPS_R_TYPE(*r.rel);
> -		if (type < ARRAY_SIZE(reloc_handlers))
> -			handler = reloc_handlers[type];
> -		else
> -			handler = NULL;
> -
> -		if (!handler) {
> -			pr_err("%s: Unknown relocation type %u\n",
> -			       me->name, type);
> -			err = -EINVAL;
> -			goto out;
> -		}
>   
>   		if (rela) {
>   			v = sym->st_value + r.rela->r_addend;
> @@ -365,7 +360,7 @@ static int __apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
>   			r.rel = &r.rel[1];
>   		}
>   
> -		err = handler(me, location, base, v, rela);
> +		err = reloc_handler(type, me, location, base, v, rela);
>   		if (err)
>   			goto out;
>   	}

Patch
diff mbox series

diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index 3c0c3d1260c1..14f46d17500a 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -40,22 +40,13 @@  void *module_alloc(unsigned long size)
 }
 #endif
 
-static int apply_r_mips_none(struct module *me, u32 *location,
-			     u32 base, Elf_Addr v, bool rela)
-{
-	return 0;
-}
-
-static int apply_r_mips_32(struct module *me, u32 *location,
-			   u32 base, Elf_Addr v, bool rela)
+static void apply_r_mips_32(u32 *location, u32 base, Elf_Addr v)
 {
 	*location = base + v;
-
-	return 0;
 }
 
-static int apply_r_mips_26(struct module *me, u32 *location,
-			   u32 base, Elf_Addr v, bool rela)
+static int apply_r_mips_26(struct module *me, u32 *location, u32 base,
+			   Elf_Addr v)
 {
 	if (v % 4) {
 		pr_err("module %s: dangerous R_MIPS_26 relocation\n",
@@ -75,8 +66,8 @@  static int apply_r_mips_26(struct module *me, u32 *location,
 	return 0;
 }
 
-static int apply_r_mips_hi16(struct module *me, u32 *location,
-			     u32 base, Elf_Addr v, bool rela)
+static int apply_r_mips_hi16(struct module *me, u32 *location, Elf_Addr v,
+			     bool rela)
 {
 	struct mips_hi16 *n;
 
@@ -217,26 +208,25 @@  static int apply_r_mips_pc(struct module *me, u32 *location, u32 base,
 	return 0;
 }
 
-static int apply_r_mips_pc16(struct module *me, u32 *location,
-			     u32 base, Elf_Addr v, bool rela)
+static int apply_r_mips_pc16(struct module *me, u32 *location, u32 base,
+			     Elf_Addr v)
 {
 	return apply_r_mips_pc(me, location, base, v, 16);
 }
 
-static int apply_r_mips_pc21(struct module *me, u32 *location,
-			     u32 base, Elf_Addr v, bool rela)
+static int apply_r_mips_pc21(struct module *me, u32 *location, u32 base,
+			     Elf_Addr v)
 {
 	return apply_r_mips_pc(me, location, base, v, 21);
 }
 
-static int apply_r_mips_pc26(struct module *me, u32 *location,
-			     u32 base, Elf_Addr v, bool rela)
+static int apply_r_mips_pc26(struct module *me, u32 *location, u32 base,
+			     Elf_Addr v)
 {
 	return apply_r_mips_pc(me, location, base, v, 26);
 }
 
-static int apply_r_mips_64(struct module *me, u32 *location,
-			   u32 base, Elf_Addr v, bool rela)
+static int apply_r_mips_64(u32 *location, Elf_Addr v, bool rela)
 {
 	if (WARN_ON(!rela))
 		return -EINVAL;
@@ -246,8 +236,7 @@  static int apply_r_mips_64(struct module *me, u32 *location,
 	return 0;
 }
 
-static int apply_r_mips_higher(struct module *me, u32 *location,
-			       u32 base, Elf_Addr v, bool rela)
+static int apply_r_mips_higher(u32 *location, Elf_Addr v, bool rela)
 {
 	if (WARN_ON(!rela))
 		return -EINVAL;
@@ -258,8 +247,7 @@  static int apply_r_mips_higher(struct module *me, u32 *location,
 	return 0;
 }
 
-static int apply_r_mips_highest(struct module *me, u32 *location,
-				u32 base, Elf_Addr v, bool rela)
+static int apply_r_mips_highest(u32 *location, Elf_Addr v, bool rela)
 {
 	if (WARN_ON(!rela))
 		return -EINVAL;
@@ -272,12 +260,14 @@  static int apply_r_mips_highest(struct module *me, u32 *location,
 
 /**
  * reloc_handler() - Apply a particular relocation to a module
+ * @type: type of the relocation to apply
  * @me: the module to apply the reloc to
  * @location: the address at which the reloc is to be applied
  * @base: the existing value at location for REL-style; 0 for RELA-style
  * @v: the value of the reloc, with addend for RELA-style
+ * @rela: indication of is this a RELA (true) or REL (false) relocation
  *
- * Each implemented reloc_handler function applies a particular type of
+ * Each implemented relocation function applies a particular type of
  * relocation to the module @me. Relocs that may be found in either REL or RELA
  * variants can be handled by making use of the @base & @v parameters which are
  * set to values which abstract the difference away from the particular reloc
@@ -285,23 +275,40 @@  static int apply_r_mips_highest(struct module *me, u32 *location,
  *
  * Return: 0 upon success, else -ERRNO
  */
-typedef int (*reloc_handler)(struct module *me, u32 *location,
-			     u32 base, Elf_Addr v, bool rela);
-
-/* The handlers for known reloc types */
-static reloc_handler reloc_handlers[] = {
-	[R_MIPS_NONE]		= apply_r_mips_none,
-	[R_MIPS_32]		= apply_r_mips_32,
-	[R_MIPS_26]		= apply_r_mips_26,
-	[R_MIPS_HI16]		= apply_r_mips_hi16,
-	[R_MIPS_LO16]		= apply_r_mips_lo16,
-	[R_MIPS_PC16]		= apply_r_mips_pc16,
-	[R_MIPS_64]		= apply_r_mips_64,
-	[R_MIPS_HIGHER]		= apply_r_mips_higher,
-	[R_MIPS_HIGHEST]	= apply_r_mips_highest,
-	[R_MIPS_PC21_S2]	= apply_r_mips_pc21,
-	[R_MIPS_PC26_S2]	= apply_r_mips_pc26,
-};
+static int reloc_handler(u32 type, struct module *me, u32 *location, u32 base,
+			 Elf_Addr v, bool rela)
+{
+	switch (type) {
+	case R_MIPS_NONE:
+		break;
+	case R_MIPS_32:
+		apply_r_mips_32(location, base, v);
+		break;
+	case R_MIPS_26:
+		return apply_r_mips_26(me, location, base, v);
+	case R_MIPS_HI16:
+		return apply_r_mips_hi16(me, location, v, rela);
+	case R_MIPS_LO16:
+		return apply_r_mips_lo16(me, location, base, v, rela);
+	case R_MIPS_PC16:
+		return apply_r_mips_pc16(me, location, base, v);
+	case R_MIPS_PC21_S2:
+		return apply_r_mips_pc21(me, location, base, v);
+	case R_MIPS_PC26_S2:
+		return apply_r_mips_pc26(me, location, base, v);
+	case R_MIPS_64:
+		return apply_r_mips_64(location, v, rela);
+	case R_MIPS_HIGHER:
+		return apply_r_mips_higher(location, v, rela);
+	case R_MIPS_HIGHEST:
+		return apply_r_mips_highest(location, v, rela);
+	default:
+		pr_err("%s: Unknown relocation type %u\n", me->name, type);
+		return -EINVAL;
+	}
+
+	return 0;
+}
 
 static int __apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
 			    unsigned int symindex, unsigned int relsec,
@@ -311,7 +318,6 @@  static int __apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
 		Elf_Mips_Rel *rel;
 		Elf_Mips_Rela *rela;
 	} r;
-	reloc_handler handler;
 	Elf_Sym *sym;
 	u32 *location, base;
 	unsigned int i, type;
@@ -343,17 +349,6 @@  static int __apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
 		}
 
 		type = ELF_MIPS_R_TYPE(*r.rel);
-		if (type < ARRAY_SIZE(reloc_handlers))
-			handler = reloc_handlers[type];
-		else
-			handler = NULL;
-
-		if (!handler) {
-			pr_err("%s: Unknown relocation type %u\n",
-			       me->name, type);
-			err = -EINVAL;
-			goto out;
-		}
 
 		if (rela) {
 			v = sym->st_value + r.rela->r_addend;
@@ -365,7 +360,7 @@  static int __apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
 			r.rel = &r.rel[1];
 		}
 
-		err = handler(me, location, base, v, rela);
+		err = reloc_handler(type, me, location, base, v, rela);
 		if (err)
 			goto out;
 	}