[PATCH] riscv: optimize ELF relocation function in riscv

[PATCH] riscv: optimize ELF relocation function in riscv

[Amma.Lee]

The patch can optimize the running times of "insmod" command by modify ELF
relocation function.
In the riscv kernel, when install the ELF driver which contains multiple
symbol table items to be relocated, kernel takes a lot of time to
execute the relocation. For example, we install a 3+MB driver need 180+s.

We focus on the riscv kernel handle R_RISCV_HI20 and R_RISCV_LO12 type
items relocation function and find that there are two for-loops in this
function. If we modify the begin number in the second for-loops iteration,
we could save significant time for installation. We install the 3+MB
driver could just need 2s.

Signed-off-by: Amma.Lee<lixiaoyun@binary-semi.com>
---
 arch/riscv/kernel/module.c | 69 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 61 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 7c651d5..55507b0 100755
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -345,13 +345,13 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 	int (*handler)(struct module *me, u32 *location, Elf_Addr v);
 	Elf_Sym *sym;
 	u32 *location;
-	unsigned int i, type;
+	unsigned int i, type, j_idx;
 	Elf_Addr v;
 	int res;
 
 	pr_debug("Applying relocate section %u to %u\n", relsec,
 	       sechdrs[relsec].sh_info);
-
+	j_idx = 0;
 	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
 		/* This is where to make the change */
 		location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
@@ -385,8 +385,9 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 
 		if (type == R_RISCV_PCREL_LO12_I || type == R_RISCV_PCREL_LO12_S) {
 			unsigned int j;
-
-			for (j = 0; j < sechdrs[relsec].sh_size / sizeof(*rel); j++) {
+			/*Modify the j for-loops begin number from last iterates end value*/
+			for (j = j_idx; j < sechdrs[relsec].sh_size / sizeof(*rel); j++) {
+			/* Modify end */
 				unsigned long hi20_loc =
 					sechdrs[sechdrs[relsec].sh_info].sh_addr
 					+ rel[j].r_offset;
@@ -420,11 +421,63 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 				}
 			}
 			if (j == sechdrs[relsec].sh_size / sizeof(*rel)) {
-				pr_err(
-				  "%s: Can not find HI20 relocation information\n",
-				  me->name);
-				return -EINVAL;
+				if (j_idx == 0) {
+					pr_err(
+						"%s: Can not find HI20 relocation information\n",
+						me->name);
+					return -EINVAL;
+				}
+
+
+				for (j = 0; j < j_idx; j++) {
+					unsigned long hi20_loc =
+						sechdrs[sechdrs[relsec].sh_info].sh_addr
+						+ rel[j].r_offset;
+					u32 hi20_type = ELF_RISCV_R_TYPE(rel[j].r_info);
+
+
+					/* Find the corresponding HI20 relocation entry */
+					if (hi20_loc == sym->st_value
+						&& (hi20_type == R_RISCV_PCREL_HI20
+						|| hi20_type == R_RISCV_GOT_HI20)) {
+						s32 hi20, lo12;
+						Elf_Sym *hi20_sym =
+							(Elf_Sym *)sechdrs[symindex].sh_addr
+							+ ELF_RISCV_R_SYM(rel[j].r_info);
+						unsigned long hi20_sym_val =
+							hi20_sym->st_value
+							+ rel[j].r_addend;
+
+
+						/* Calculate lo12 */
+						size_t offset = hi20_sym_val - hi20_loc;
+						/* Calculate lo12 */
+						if (IS_ENABLED(CONFIG_MODULE_SECTIONS)
+							&& hi20_type == R_RISCV_GOT_HI20) {
+							offset = module_emit_got_entry(
+								me, hi20_sym_val);
+							offset = offset - hi20_loc;
+
+						}
+						hi20 = (offset + 0x800) & 0xfffff000;
+						lo12 = offset - hi20;
+						v = lo12;
+
+						break;
+					}
+				}
+
+				if (j == j_idx) {
+					pr_err(
+						"%s: Can not find HI20 relocation information\n",
+						me->name);
+					return -EINVAL;
+				}
+
+
 			}
+
+			j_idx = j;
 		}
 
 		res = handler(me, location, v);
-- 
1.9.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: optimize ELF relocation function in riscv

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2152 bytes --]

Hey Amma,

The patchwork automation seems to have skipped this patch for some
reason, so here I am doing it manually instead!

On Wed, May 10, 2023 at 07:10:18PM +0800, Amma.Lee wrote:
> The patch can optimize the running times of "insmod" command by modify ELF
> relocation function.
> In the riscv kernel, when install the ELF driver which contains multiple
> symbol table items to be relocated, kernel takes a lot of time to
> execute the relocation. For example, we install a 3+MB driver need 180+s.
> 
> We focus on the riscv kernel handle R_RISCV_HI20 and R_RISCV_LO12 type
> items relocation function and find that there are two for-loops in this
> function. If we modify the begin number in the second for-loops iteration,
> we could save significant time for installation. We install the 3+MB
> driver could just need 2s.
> 
> Signed-off-by: Amma.Lee<lixiaoyun@binary-semi.com>

Firstly, please remove the . between your names, and add a space before
the <.
`git commit -s` will automagically add a SoB FYI.

When I applied this patch I got:
Applying: riscv: optimize ELF relocation function in riscv
warning: arch/riscv/kernel/module.c has type 100644, expected 100755

There are also quite a lot of checkpatch coding-style issues:
| 65: CHECK: Lines should not end with a '('
| 71: CHECK: Please don't use multiple blank lines
| 78: CHECK: Please don't use multiple blank lines
| 81: CHECK: Logical continuations should be on the previous line
| 82: CHECK: Logical continuations should be on the previous line
| 91: CHECK: Please don't use multiple blank lines
| 95: WARNING: Too many leading tabs - consider code refactoring
| 96: CHECK: Logical continuations should be on the previous line
| 97: CHECK: Lines should not end with a '('
| 101: CHECK: Blank lines aren't necessary before a close brace '}'
| 111: CHECK: Lines should not end with a '('
| 117: CHECK: Please don't use multiple blank lines
| total: 0 errors, 1 warnings, 11 checks, 93 lines checked

Hopefully you get some comments on the code itself, and when you resend,
the automation does actually pick it up.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH] riscv: optimize ELF relocation function in riscv

[Amma Lee]

The patch can optimize the running times of insmod command by modify ELF
relocation function.
In the 5.10 and latest kernel, when install the riscv ELF drivers which
contains multiple symbol table items to be relocated, kernel takes a lot
of time to execute the relocation. For example, we install a 3+MB driver
need 180+s.
We focus on the riscv architecture handle R_RISCV_HI20 and R_RISCV_LO20
type items relocation function in the arch\riscv\kernel\module.c and
find that there are two-loops in the function. If we modify the begin
number in the second for-loops iteration, we could save significant time
for installation. We install the same 3+MB driver could just need 2s.

Signed-off-by: Amma Lee <lixiaoyun@binary-semi.com>
---
 arch/riscv/kernel/module.c | 67 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 7c651d5..b8df144 100755
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -345,13 +345,13 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 	int (*handler)(struct module *me, u32 *location, Elf_Addr v);
 	Elf_Sym *sym;
 	u32 *location;
-	unsigned int i, type;
+	unsigned int i, type, j_idx;
 	Elf_Addr v;
 	int res;
 
 	pr_debug("Applying relocate section %u to %u\n", relsec,
 	       sechdrs[relsec].sh_info);
-
+	j_idx = 0;
 	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
 		/* This is where to make the change */
 		location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
@@ -386,7 +386,15 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 		if (type == R_RISCV_PCREL_LO12_I || type == R_RISCV_PCREL_LO12_S) {
 			unsigned int j;
 
-			for (j = 0; j < sechdrs[relsec].sh_size / sizeof(*rel); j++) {
+			/*In the second for-loops, each traversal for j is
+			 * starts from 0 to the symbol table item index which
+			 * is detected. By the tool "readelf", we find that all
+			 * the symbol table items about R_RISCV_PCREL_HI20 type
+			 * are incrementally added in order. It means that we
+			 * could interate the j with the previous loop end
+			 * value(j_idx) as the begin number in the next loop;
+			 */
+			for (j = j_idx; j < sechdrs[relsec].sh_size / sizeof(*rel); j++) {
 				unsigned long hi20_loc =
 					sechdrs[sechdrs[relsec].sh_info].sh_addr
 					+ rel[j].r_offset;
@@ -420,11 +428,64 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 				}
 			}
 			if (j == sechdrs[relsec].sh_size / sizeof(*rel)) {
+				if (j_idx == 0) {
 				pr_err(
 				  "%s: Can not find HI20 relocation information\n",
 				  me->name);
 				return -EINVAL;
+}
+
+				/*If the last j-loop have been traversed to the
+				 * maximum value but never match the
+				 * corresponding symbol relocation item, the
+				 * j-loop will execute the second loop which
+				 * is begin from 0 to the prerious index (j_idx)
+				 * unless the previous j_idx == 0;
+				 * */
+				for (j = 0; j < j_idx; j++) {
+				unsigned long hi20_loc =
+				sechdrs[sechdrs[relsec].sh_info].sh_addr
+						+ rel[j].r_offset;
+				u32 hi20_type = ELF_RISCV_R_TYPE(rel[j].r_info);
+
+				/* Find the corresponding HI20 relocation entry */
+				if (hi20_loc == sym->st_value
+					&& (hi20_type == R_RISCV_PCREL_HI20
+					|| hi20_type == R_RISCV_GOT_HI20)) {
+					s32 hi20, lo12;
+					Elf_Sym *hi20_sym =
+						(Elf_Sym *)sechdrs[symindex].sh_addr
+						+ ELF_RISCV_R_SYM(rel[j].r_info);
+					unsigned long hi20_sym_val =
+						hi20_sym->st_value
+						+ rel[j].r_addend;
+
+					/* Calculate lo12 */
+					size_t offset = hi20_sym_val - hi20_loc;
+					if (IS_ENABLED(CONFIG_MODULE_SECTIONS)
+					    && hi20_type == R_RISCV_GOT_HI20) {
+						offset = module_emit_got_entry(
+							 me, hi20_sym_val);
+						offset = offset - hi20_loc;
+					}
+					hi20 = (offset + 0x800) & 0xfffff000;
+					lo12 = offset - hi20;
+					v = lo12;
+
+					break;
+				}
 			}
+
+				if (j == j_idx) {
+					pr_err(
+						"%s: Can not find HI20 relocation information\n",
+						me->name);
+					return -EINVAL;
+				}
+			}
+
+			/* Record the previous j-loop end index */
+			j_idx = j;
 		}
 
 		res = handler(me, location, v);
-- 
1.9.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: optimize ELF relocation function in riscv

[Andrew Jones]

On Mon, Jul 03, 2023 at 11:32:12AM +0800, Amma Lee wrote:
> The patch can optimize the running times of insmod command by modify ELF
> relocation function.
> In the 5.10 and latest kernel, when install the riscv ELF drivers which
> contains multiple symbol table items to be relocated, kernel takes a lot
> of time to execute the relocation. For example, we install a 3+MB driver
> need 180+s.
> We focus on the riscv architecture handle R_RISCV_HI20 and R_RISCV_LO20
> type items relocation function in the arch\riscv\kernel\module.c and
> find that there are two-loops in the function. If we modify the begin
> number in the second for-loops iteration, we could save significant time
> for installation. We install the same 3+MB driver could just need 2s.
> 
> Signed-off-by: Amma Lee <lixiaoyun@binary-semi.com>
> ---
>  arch/riscv/kernel/module.c | 67 +++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 64 insertions(+), 3 deletions(-)
>

I guess this is a v3 of [1]? But there's no change log here to know
what's different.

[1] 1683881513-18730-1-git-send-email-lixiaoyun@binary-semi.com

Thanks,
drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: optimize ELF relocation function in riscv

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1466 bytes --]

On Mon, Jul 03, 2023 at 12:35:15PM +0200, Andrew Jones wrote:
> On Mon, Jul 03, 2023 at 11:32:12AM +0800, Amma Lee wrote:
> > The patch can optimize the running times of insmod command by modify ELF
> > relocation function.
> > In the 5.10 and latest kernel, when install the riscv ELF drivers which
> > contains multiple symbol table items to be relocated, kernel takes a lot
> > of time to execute the relocation. For example, we install a 3+MB driver
> > need 180+s.
> > We focus on the riscv architecture handle R_RISCV_HI20 and R_RISCV_LO20
> > type items relocation function in the arch\riscv\kernel\module.c and
> > find that there are two-loops in the function. If we modify the begin
> > number in the second for-loops iteration, we could save significant time
> > for installation. We install the same 3+MB driver could just need 2s.
> > 
> > Signed-off-by: Amma Lee <lixiaoyun@binary-semi.com>
> > ---
> >  arch/riscv/kernel/module.c | 67 +++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 64 insertions(+), 3 deletions(-)
> >
> 
> I guess this is a v3 of [1]? But there's no change log here to know
> what's different.
> 
> [1] 1683881513-18730-1-git-send-email-lixiaoyun@binary-semi.com

It's also still got the checkpatch issues (and possibly others) that
were pointed out previously.

Cheers,
Conor.

Also, when applying the patch:
warning: arch/riscv/kernel/module.c has type 100644, expected 100755

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv