2014-07-22 12:28:32

by Petr Mladek

Subject: [PATCH 0/2] module: add within_module() and change return type

The condition (within_module_init() || within_module_core()) is used on several
locations. We would like to use it also in kGraft when patching modules.

This small patch set introduces within_module() to do the check in one call.
It also modifies the return type from int to bool for all three functions.
It seems to be more appropriate.

The patch set is against linux-next, top commit 594a8bbcd415c ("Add linux-next
specific files for 20140721").

Petr Mladek (2):
module: add within_module() function
module: return bool from within_module*()

include/linux/module.h | 11 +++++++++--
kernel/module.c | 12 ++++--------
2 files changed, 13 insertions(+), 10 deletions(-)

--
1.8.4

2014-07-22 12:28:51

by Petr Mladek

Subject: [PATCH 1/2] module: add within_module() function

It is just a small optimization that allows to replace few
occurrences of within_module_init() || within_module_core()
with a single call.

Signed-off-by: Petr Mladek <[email protected]>
---
include/linux/module.h | 5 +++++
kernel/module.c | 12 ++++--------
2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index f520a767c86c..61d8fb2d0873 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -408,6 +408,11 @@ static inline int within_module_init(unsigned long addr, const struct module *mo
addr < (unsigned long)mod->module_init + mod->init_size;
}

+static inline int within_module(unsigned long addr, const struct module *mod)
+{
+ return within_module_init(addr, mod) || within_module_core(addr, mod);
+}
+
/* Search for module by name: must hold module_mutex. */
struct module *find_module(const char *name);

diff --git a/kernel/module.c b/kernel/module.c
index ae79ce615cb9..be0e479ccb5c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3444,8 +3444,7 @@ const char *module_address_lookup(unsigned long addr,
list_for_each_entry_rcu(mod, &modules, list) {
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- if (within_module_init(addr, mod) ||
- within_module_core(addr, mod)) {
+ if (within_module(addr, mod)) {
if (modname)
*modname = mod->name;
ret = get_ksymbol(mod, addr, size, offset);
@@ -3469,8 +3468,7 @@ int lookup_module_symbol_name(unsigned long addr, char *symname)
list_for_each_entry_rcu(mod, &modules, list) {
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- if (within_module_init(addr, mod) ||
- within_module_core(addr, mod)) {
+ if (within_module(addr, mod)) {
const char *sym;

sym = get_ksymbol(mod, addr, NULL, NULL);
@@ -3495,8 +3493,7 @@ int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,
list_for_each_entry_rcu(mod, &modules, list) {
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- if (within_module_init(addr, mod) ||
- within_module_core(addr, mod)) {
+ if (within_module(addr, mod)) {
const char *sym;

sym = get_ksymbol(mod, addr, size, offset);
@@ -3760,8 +3757,7 @@ struct module *__module_address(unsigned long addr)
list_for_each_entry_rcu(mod, &modules, list) {
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- if (within_module_core(addr, mod)
- || within_module_init(addr, mod))
+ if (within_module(addr, mod))
return mod;
}
return NULL;
--
1.8.4

2014-07-22 12:29:13

by Petr Mladek

Subject: [PATCH 2/2] module: return bool from within_module*()

The within_module*() functions return only true or false. Let's use bool as
the return type.

Note that it should not change kABI because these are inline functions.

Signed-off-by: Petr Mladek <[email protected]>
---
include/linux/module.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 61d8fb2d0873..71f282a4e307 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -396,19 +396,21 @@ bool is_module_address(unsigned long addr);
bool is_module_percpu_address(unsigned long addr);
bool is_module_text_address(unsigned long addr);

-static inline int within_module_core(unsigned long addr, const struct module *mod)
+static inline bool within_module_core(unsigned long addr,
+ const struct module *mod)
{
return (unsigned long)mod->module_core <= addr &&
addr < (unsigned long)mod->module_core + mod->core_size;
}

-static inline int within_module_init(unsigned long addr, const struct module *mod)
+static inline bool within_module_init(unsigned long addr,
+ const struct module *mod)
{
return (unsigned long)mod->module_init <= addr &&
addr < (unsigned long)mod->module_init + mod->init_size;
}

-static inline int within_module(unsigned long addr, const struct module *mod)
+static inline bool within_module(unsigned long addr, const struct module *mod)
{
return within_module_init(addr, mod) || within_module_core(addr, mod);
}
--
1.8.4

2014-07-22 12:40:49

by Steven Rostedt

Subject: Re: [PATCH 1/2] module: add within_module() function

On Tue, 22 Jul 2014 14:28:09 +0200
Petr Mladek <[email protected]> wrote:

> It is just a small optimization that allows to replace few
> occurrences of within_module_init() || within_module_core()
> with a single call.

This looks like a nice clean up. Rusty, what do you think?

-- Steve

>
> Signed-off-by: Petr Mladek <[email protected]>
> ---
> include/linux/module.h | 5 +++++
> kernel/module.c | 12 ++++--------
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index f520a767c86c..61d8fb2d0873 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -408,6 +408,11 @@ static inline int within_module_init(unsigned long addr, const struct module *mo
> addr < (unsigned long)mod->module_init + mod->init_size;
> }
>
> +static inline int within_module(unsigned long addr, const struct module *mod)
> +{
> + return within_module_init(addr, mod) || within_module_core(addr, mod);
> +}
> +
> /* Search for module by name: must hold module_mutex. */
> struct module *find_module(const char *name);
>
> diff --git a/kernel/module.c b/kernel/module.c
> index ae79ce615cb9..be0e479ccb5c 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3444,8 +3444,7 @@ const char *module_address_lookup(unsigned long addr,
> list_for_each_entry_rcu(mod, &modules, list) {
> if (mod->state == MODULE_STATE_UNFORMED)
> continue;
> - if (within_module_init(addr, mod) ||
> - within_module_core(addr, mod)) {
> + if (within_module(addr, mod)) {
> if (modname)
> *modname = mod->name;
> ret = get_ksymbol(mod, addr, size, offset);
> @@ -3469,8 +3468,7 @@ int lookup_module_symbol_name(unsigned long addr, char *symname)
> list_for_each_entry_rcu(mod, &modules, list) {
> if (mod->state == MODULE_STATE_UNFORMED)
> continue;
> - if (within_module_init(addr, mod) ||
> - within_module_core(addr, mod)) {
> + if (within_module(addr, mod)) {
> const char *sym;
>
> sym = get_ksymbol(mod, addr, NULL, NULL);
> @@ -3495,8 +3493,7 @@ int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,
> list_for_each_entry_rcu(mod, &modules, list) {
> if (mod->state == MODULE_STATE_UNFORMED)
> continue;
> - if (within_module_init(addr, mod) ||
> - within_module_core(addr, mod)) {
> + if (within_module(addr, mod)) {
> const char *sym;
>
> sym = get_ksymbol(mod, addr, size, offset);
> @@ -3760,8 +3757,7 @@ struct module *__module_address(unsigned long addr)
> list_for_each_entry_rcu(mod, &modules, list) {
> if (mod->state == MODULE_STATE_UNFORMED)
> continue;
> - if (within_module_core(addr, mod)
> - || within_module_init(addr, mod))
> + if (within_module(addr, mod))
> return mod;
> }
> return NULL;

2014-07-23 05:56:37

by Rusty Russell

Subject: Re: [PATCH 2/2] module: return bool from within_module*()

Petr Mladek <[email protected]> writes:
> The within_module*() functions return only true or false. Let's use bool as
> the return type.
>
> Note that it should not change kABI because these are inline functions.
>
> Signed-off-by: Petr Mladek <[email protected]>

Thanks, applied both.

Cheers,
Rusty.

> ---
> include/linux/module.h | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 61d8fb2d0873..71f282a4e307 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -396,19 +396,21 @@ bool is_module_address(unsigned long addr);
> bool is_module_percpu_address(unsigned long addr);
> bool is_module_text_address(unsigned long addr);
>
> -static inline int within_module_core(unsigned long addr, const struct module *mod)
> +static inline bool within_module_core(unsigned long addr,
> + const struct module *mod)
> {
> return (unsigned long)mod->module_core <= addr &&
> addr < (unsigned long)mod->module_core + mod->core_size;
> }
>
> -static inline int within_module_init(unsigned long addr, const struct module *mod)
> +static inline bool within_module_init(unsigned long addr,
> + const struct module *mod)
> {
> return (unsigned long)mod->module_init <= addr &&
> addr < (unsigned long)mod->module_init + mod->init_size;
> }
>
> -static inline int within_module(unsigned long addr, const struct module *mod)
> +static inline bool within_module(unsigned long addr, const struct module *mod)
> {
> return within_module_init(addr, mod) || within_module_core(addr, mod);
> }
> --
> 1.8.4