Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1576666 > unrolled thread
| Started by | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| First post | 2017-02-08 17:00 +0100 |
| Last post | 2017-02-11 05:50 +0100 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] module: Optimize search_module_extables() Peter Zijlstra <peterz@infradead.org> - 2017-02-08 17:00 +0100
Re: [PATCH] module: Optimize search_module_extables() Mark Rutland <mark.rutland@arm.com> - 2017-02-08 17:10 +0100
Re: module: Optimize search_module_extables() Jessica Yu <jeyu@redhat.com> - 2017-02-11 05:50 +0100
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-02-08 17:00 +0100 |
| Subject | [PATCH] module: Optimize search_module_extables() |
| Message-ID | <t8COZ-2b0-1@gated-at.bofh.it> |
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;
}
[toc] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-02-08 17:10 +0100 |
| Message-ID | <t8CYF-2tE-5@gated-at.bofh.it> |
| In reply to | #1576666 |
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;
> }
>
[toc] | [prev] | [next] | [standalone]
| From | Jessica Yu <jeyu@redhat.com> |
|---|---|
| Date | 2017-02-11 05:50 +0100 |
| Subject | Re: module: Optimize search_module_extables() |
| Message-ID | <t9xNf-4xU-3@gated-at.bofh.it> |
| In reply to | #1576666 |
+++ 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;
> }
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web