[PATCH] module: Optimize search_module_extables()

[PATCH] module: Optimize search_module_extables()

[Peter Zijlstra]

While looking through the __ex_table stuff I found that we do a linear
lookup of the module. Also fix up a comment.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/module.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 3d8f126208e3..7bcdc35dbf95 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4165,22 +4165,23 @@ const struct exception_table_entry *search_module_extables(unsigned long addr)
 	struct module *mod;
 
 	preempt_disable();
-	list_for_each_entry_rcu(mod, &modules, list) {
-		if (mod->state == MODULE_STATE_UNFORMED)
-			continue;
-		if (mod->num_exentries == 0)
-			continue;
+	mod = __module_address(addr);
+	if (!mod)
+		goto out;
 
-		e = search_extable(mod->extable,
-				   mod->extable + mod->num_exentries - 1,
-				   addr);
-		if (e)
-			break;
-	}
+	if (!mod->num_exentries)
+		goto out;
+
+	e = search_extable(mod->extable,
+			   mod->extable + mod->num_exentries - 1,
+			   addr);
+out:
 	preempt_enable();
 
-	/* Now, if we found one, we are running inside it now, hence
-	   we cannot unload the module, hence no refcnt needed. */
+	/*
+	 * Now, if we found one, we are running inside it now, hence
+	 * we cannot unload the module, hence no refcnt needed.
+	 */
 	return e;
 }
 

Re: [PATCH] module: Optimize search_module_extables()

[Mark Rutland]

On Wed, Feb 08, 2017 at 03:48:01PM +0100, Peter Zijlstra wrote:
> 
> While looking through the __ex_table stuff I found that we do a linear
> lookup of the module. Also fix up a comment.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

I'm not all that familiar with the module code, but from a quick scan of
__module_address() this looks sane to me. FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  kernel/module.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 3d8f126208e3..7bcdc35dbf95 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -4165,22 +4165,23 @@ const struct exception_table_entry *search_module_extables(unsigned long addr)
>  	struct module *mod;
>  
>  	preempt_disable();
> -	list_for_each_entry_rcu(mod, &modules, list) {
> -		if (mod->state == MODULE_STATE_UNFORMED)
> -			continue;
> -		if (mod->num_exentries == 0)
> -			continue;
> +	mod = __module_address(addr);
> +	if (!mod)
> +		goto out;
>  
> -		e = search_extable(mod->extable,
> -				   mod->extable + mod->num_exentries - 1,
> -				   addr);
> -		if (e)
> -			break;
> -	}
> +	if (!mod->num_exentries)
> +		goto out;
> +
> +	e = search_extable(mod->extable,
> +			   mod->extable + mod->num_exentries - 1,
> +			   addr);
> +out:
>  	preempt_enable();
>  
> -	/* Now, if we found one, we are running inside it now, hence
> -	   we cannot unload the module, hence no refcnt needed. */
> +	/*
> +	 * Now, if we found one, we are running inside it now, hence
> +	 * we cannot unload the module, hence no refcnt needed.
> +	 */
>  	return e;
>  }
>  

Re: module: Optimize search_module_extables()

[Jessica Yu]

+++ Peter Zijlstra [08/02/17 15:48 +0100]:
>
>While looking through the __ex_table stuff I found that we do a linear
>lookup of the module. Also fix up a comment.
>
>Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Applied, thanks.

Hm. A quick scan through module.c still shows a couple of places that use
similar linear lookups, and may benefit from the same __module_address
optimization. But I'll save that for a separate patch..

Jessica

>---
> kernel/module.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index 3d8f126208e3..7bcdc35dbf95 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -4165,22 +4165,23 @@ const struct exception_table_entry *search_module_extables(unsigned long addr)
> 	struct module *mod;
>
> 	preempt_disable();
>-	list_for_each_entry_rcu(mod, &modules, list) {
>-		if (mod->state == MODULE_STATE_UNFORMED)
>-			continue;
>-		if (mod->num_exentries == 0)
>-			continue;
>+	mod = __module_address(addr);
>+	if (!mod)
>+		goto out;
>
>-		e = search_extable(mod->extable,
>-				   mod->extable + mod->num_exentries - 1,
>-				   addr);
>-		if (e)
>-			break;
>-	}
>+	if (!mod->num_exentries)
>+		goto out;
>+
>+	e = search_extable(mod->extable,
>+			   mod->extable + mod->num_exentries - 1,
>+			   addr);
>+out:
> 	preempt_enable();
>
>-	/* Now, if we found one, we are running inside it now, hence
>-	   we cannot unload the module, hence no refcnt needed. */
>+	/*
>+	 * Now, if we found one, we are running inside it now, hence
>+	 * we cannot unload the module, hence no refcnt needed.
>+	 */
> 	return e;
> }
>