[PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]"

[PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]"

[liqiong]

"char bdi_unknown_nam[]" string form declares a single variable.
It is better then "char *bdi_unknown_name" which creates two
variables.

Signed-off-by: liqiong <liqiong@nfschina.com>
---
 mm/backing-dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 7176af65b103..4982ccc63536 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -20,7 +20,7 @@ struct backing_dev_info noop_backing_dev_info;
 EXPORT_SYMBOL_GPL(noop_backing_dev_info);
 
 static struct class *bdi_class;
-static const char *bdi_unknown_name = "(unknown)";
+static const char bdi_unknown_name[] = "(unknown)";
 
 /*
  * bdi_lock protects bdi_tree and updates to bdi_list. bdi_list has RCU
-- 
2.11.0

[PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]"

[liqiong]

"char bdi_unknown_nam[]" string form declares a single variable.
It is better than "char *bdi_unknown_name" which creates two
variables.

Signed-off-by: liqiong <liqiong@nfschina.com>
---
 mm/backing-dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 7176af65b103..4982ccc63536 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -20,7 +20,7 @@ struct backing_dev_info noop_backing_dev_info;
 EXPORT_SYMBOL_GPL(noop_backing_dev_info);
 
 static struct class *bdi_class;
-static const char *bdi_unknown_name = "(unknown)";
+static const char bdi_unknown_name[] = "(unknown)";
 
 /*
  * bdi_lock protects bdi_tree and updates to bdi_list. bdi_list has RCU
-- 
2.11.0

Re: [PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]"

[Muchun Song]

On Thu, May 12, 2022 at 04:26:37PM +0800, liqiong wrote:
> "char bdi_unknown_nam[]" string form declares a single variable.
> It is better then "char *bdi_unknown_name" which creates two
> variables.
>

Sorry, I do not understand what you are saying here.  Creating
two variables means what?

Thanks.
 
> Signed-off-by: liqiong <liqiong@nfschina.com>
> ---
>  mm/backing-dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index 7176af65b103..4982ccc63536 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -20,7 +20,7 @@ struct backing_dev_info noop_backing_dev_info;
>  EXPORT_SYMBOL_GPL(noop_backing_dev_info);
>  
>  static struct class *bdi_class;
> -static const char *bdi_unknown_name = "(unknown)";
> +static const char bdi_unknown_name[] = "(unknown)";
>  
>  /*
>   * bdi_lock protects bdi_tree and updates to bdi_list. bdi_list has RCU
> -- 
> 2.11.0
> 
> 

Re: [PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]"

[liqiong]

在 2022年05月12日 23:29, Muchun Song 写道:
> On Thu, May 12, 2022 at 04:26:37PM +0800, liqiong wrote:
>> "char bdi_unknown_nam[]" string form declares a single variable.
>> It is better then "char *bdi_unknown_name" which creates two
>> variables.
>>
> Sorry, I do not understand what you are saying here.  Creating
> two variables means what?
>
> Thanks.

Hi there，

The string form of "char *" creates two variables in the final assembly output, 
a static string, and a char pointer to the static string.

Use "objdump -S -D *.o", can find out the static string occurring at 
"Contents of section .rodata".


>  
>> Signed-off-by: liqiong <liqiong@nfschina.com>
>> ---
>>  mm/backing-dev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>> index 7176af65b103..4982ccc63536 100644
>> --- a/mm/backing-dev.c
>> +++ b/mm/backing-dev.c
>> @@ -20,7 +20,7 @@ struct backing_dev_info noop_backing_dev_info;
>>  EXPORT_SYMBOL_GPL(noop_backing_dev_info);
>>  
>>  static struct class *bdi_class;
>> -static const char *bdi_unknown_name = "(unknown)";
>> +static const char bdi_unknown_name[] = "(unknown)";
>>  
>>  /*
>>   * bdi_lock protects bdi_tree and updates to bdi_list. bdi_list has RCU
>> -- 
>> 2.11.0
>>
>>

-- 
李力琼 <13524287433>
上海市浦东新区海科路99号中科院上海高等研究院3号楼3楼

Re: [PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]"

[Andrew Morton]

On Thu, 12 May 2022 16:26:37 +0800 liqiong <liqiong@nfschina.com> wrote:

> "char bdi_unknown_nam[]" string form declares a single variable.
> It is better then "char *bdi_unknown_name" which creates two
> variables.
> 
> ...
>
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -20,7 +20,7 @@ struct backing_dev_info noop_backing_dev_info;
>  EXPORT_SYMBOL_GPL(noop_backing_dev_info);
>  
>  static struct class *bdi_class;
> -static const char *bdi_unknown_name = "(unknown)";
> +static const char bdi_unknown_name[] = "(unknown)";
>  

heh, fun patch.  We actually do this quite a lot.  

	grep -r "^[a-z].*char \*[a-z].*= \"" .

is a pathetic pattern which catches a lot of them.


However.  I expected your patch to shrink the kernel a bit, but it has
the opposite effect:

hp2:/usr/src/25> size mm/backing-dev.o
   text	   data	    bss	    dec	    hex	filename
  21288	   9396	   3808	  34492	   86bc	mm/backing-dev.o-before
  21300	   9428	   3808	  34536	   86e8	mm/backing-dev.o-after

Even .data became larger.  I didn't investigate why.