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

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

[Petr Mladek]

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

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

[Petr Mladek]

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 <pmladek@suse.cz>
---
 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

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

[Petr Mladek]

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 <pmladek@suse.cz>
---
 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

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

[Steven Rostedt]

On Tue, 22 Jul 2014 14:28:09 +0200
Petr Mladek <pmladek@suse.cz> 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 <pmladek@suse.cz>
> ---
>  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;

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

[Rusty Russell]

Petr Mladek <pmladek@suse.cz> 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 <pmladek@suse.cz>

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