From: liqiong <liqiong@nfschina.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]"
Date: Fri, 13 May 2022 14:34:56 +0800 [thread overview]
Message-ID: <2d698914-befd-0283-71ed-599aa1bf9e3d@nfschina.com> (raw)
In-Reply-To: <20220512130051.94a0c53e5d1498292473975d@linux-foundation.org>
在 2022年05月13日 04:00, 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.
Hi,
It seems the patch creates a new section:
0000000000000000 l ___ksymtab_gpl+bdi_dev_name 0000000000000000 __ksymtab_bdi_dev_name
0000000000000f60 l O .rodata 000000000000000a bdi_unknown_name
And put "bdi_unknown_name" at .rodata.str1.1:
This work was published in "KernelJanitors/Todo". It says:
"foo []" is better because it declares a single variable, For variables marked __initdata, the "*foo" form causes
only the pointer, not the string itself, to be dropped from the kernel image, which is a bug.
Using the "foo[]" form with regular 'ole local variables also makes the assembly shorter.
I thought it make sense, so i searched the mm tree by "grep -nHre char.*\*.*=.*\"", and checked all the "char *foo"
string form, It seems only this one should be fixed.
next prev parent reply other threads:[~2022-05-13 6:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-12 8:26 [PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]" liqiong
2022-05-12 14:48 ` liqiong
2022-05-12 15:29 ` Muchun Song
2022-05-12 17:10 ` liqiong
2022-05-12 20:00 ` Andrew Morton
2022-05-13 6:34 ` liqiong [this message]
2022-05-13 11:06 ` David Laight
2022-05-13 14:27 ` Muchun Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2d698914-befd-0283-71ed-599aa1bf9e3d@nfschina.com \
--to=liqiong@nfschina.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).