Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1362207 > unrolled thread

[PATCH resend] init/main.c: Simplify initcall_blacklisted()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH resend] init/main.c: Simplify initcall_blacklisted() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2016-03-22 00:20 +0100
    Re: [PATCH resend] init/main.c: Simplify initcall_blacklisted() Rusty Russell <rusty@rustcorp.com.au> - 2016-03-22 22:00 +0100
      Re: [PATCH resend] init/main.c: Simplify initcall_blacklisted() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2016-03-24 01:00 +0100
    Re: [PATCH resend] init/main.c: Simplify initcall_blacklisted() Prarit Bhargava <prarit@redhat.com> - 2016-03-24 18:20 +0100

#1362207 — [PATCH resend] init/main.c: Simplify initcall_blacklisted()

Using kasprintf to get the function name makes us look up the name
twice, along with all the vsnprintf overhead of parsing the format
string etc. It also means there is an allocation failure case to deal
with. Since symbol_string in vsprintf.c would anyway allocate an array
of size KSYM_SYMBOL_LEN on the stack, that might as well be done up
here.

Moreover, since this is a debug feature and the blacklisted_initcalls
list is usually empty, we might as well test that and thus avoid
looking up the symbol name even once in the common case.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 init/main.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/init/main.c b/init/main.c
index b3c6e363ae18..d76d94cd537c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -706,21 +706,20 @@ static int __init initcall_blacklist(char *str)
 static bool __init_or_module initcall_blacklisted(initcall_t fn)
 {
 	struct blacklist_entry *entry;
-	char *fn_name;
+	char fn_name[KSYM_SYMBOL_LEN];
 
-	fn_name = kasprintf(GFP_KERNEL, "%pf", fn);
-	if (!fn_name)
+	if (list_empty(&blacklisted_initcalls))
 		return false;
 
+	sprint_symbol_no_offset(fn_name, (unsigned long)fn);
+
 	list_for_each_entry(entry, &blacklisted_initcalls, next) {
 		if (!strcmp(fn_name, entry->buf)) {
 			pr_debug("initcall %s blacklisted\n", fn_name);
-			kfree(fn_name);
 			return true;
 		}
 	}
 
-	kfree(fn_name);
 	return false;
 }
 #else
-- 
2.1.4

[toc] | [next] | [standalone]

#1362970

Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:
> Using kasprintf to get the function name makes us look up the name
> twice, along with all the vsnprintf overhead of parsing the format
> string etc. It also means there is an allocation failure case to deal
> with. Since symbol_string in vsprintf.c would anyway allocate an array
> of size KSYM_SYMBOL_LEN on the stack, that might as well be done up
> here.
>
> Moreover, since this is a debug feature and the blacklisted_initcalls
> list is usually empty, we might as well test that and thus avoid
> looking up the symbol name even once in the common case.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks!
Rusty.

> ---
>  init/main.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index b3c6e363ae18..d76d94cd537c 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -706,21 +706,20 @@ static int __init initcall_blacklist(char *str)
>  static bool __init_or_module initcall_blacklisted(initcall_t fn)
>  {
>  	struct blacklist_entry *entry;
> -	char *fn_name;
> +	char fn_name[KSYM_SYMBOL_LEN];
>  
> -	fn_name = kasprintf(GFP_KERNEL, "%pf", fn);
> -	if (!fn_name)
> +	if (list_empty(&blacklisted_initcalls))
>  		return false;
>  
> +	sprint_symbol_no_offset(fn_name, (unsigned long)fn);
> +
>  	list_for_each_entry(entry, &blacklisted_initcalls, next) {
>  		if (!strcmp(fn_name, entry->buf)) {
>  			pr_debug("initcall %s blacklisted\n", fn_name);
> -			kfree(fn_name);
>  			return true;
>  		}
>  	}
>  
> -	kfree(fn_name);
>  	return false;
>  }
>  #else
> -- 
> 2.1.4

[toc] | [prev] | [next] | [standalone]

#1363793

On Tue, Mar 22 2016, Rusty Russell <rusty@rustcorp.com.au> wrote:

> Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:
>> Using kasprintf to get the function name makes us look up the name
>> twice, along with all the vsnprintf overhead of parsing the format
>> string etc. It also means there is an allocation failure case to deal
>> with. Since symbol_string in vsprintf.c would anyway allocate an array
>> of size KSYM_SYMBOL_LEN on the stack, that might as well be done up
>> here.
>>
>> Moreover, since this is a debug feature and the blacklisted_initcalls
>> list is usually empty, we might as well test that and thus avoid
>> looking up the symbol name even once in the common case.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>
>

Thanks. Andrew, can I get you to take it?

[toc] | [prev] | [next] | [standalone]

#1364365

On 03/21/2016 07:14 PM, Rasmus Villemoes wrote:
> Using kasprintf to get the function name makes us look up the name
> twice, along with all the vsnprintf overhead of parsing the format
> string etc. It also means there is an allocation failure case to deal
> with. Since symbol_string in vsprintf.c would anyway allocate an array
> of size KSYM_SYMBOL_LEN on the stack, that might as well be done up
> here.
> 
> Moreover, since this is a debug feature and the blacklisted_initcalls
> list is usually empty, we might as well test that and thus avoid
> looking up the symbol name even once in the common case.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>


Acked-by: Prarit Bhargava <prarit@redhat.com>

P.


> ---
>  init/main.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/init/main.c b/init/main.c
> index b3c6e363ae18..d76d94cd537c 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -706,21 +706,20 @@ static int __init initcall_blacklist(char *str)
>  static bool __init_or_module initcall_blacklisted(initcall_t fn)
>  {
>  	struct blacklist_entry *entry;
> -	char *fn_name;
> +	char fn_name[KSYM_SYMBOL_LEN];
>  
> -	fn_name = kasprintf(GFP_KERNEL, "%pf", fn);
> -	if (!fn_name)
> +	if (list_empty(&blacklisted_initcalls))
>  		return false;
>  
> +	sprint_symbol_no_offset(fn_name, (unsigned long)fn);
> +
>  	list_for_each_entry(entry, &blacklisted_initcalls, next) {
>  		if (!strcmp(fn_name, entry->buf)) {
>  			pr_debug("initcall %s blacklisted\n", fn_name);
> -			kfree(fn_name);
>  			return true;
>  		}
>  	}
>  
> -	kfree(fn_name);
>  	return false;
>  }
>  #else
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web