[PATCH 0/5] mm: Some .text savings

[PATCH 0/5] mm: Some .text savings

[Rasmus Villemoes]

Only compile-tested, but I think these should be ok. Net saving aroung
450 bytes of .text.

Rasmus Villemoes (5):
  mm/internal.h: Don't split printk call in two
  mm/page_alloc.c: Pull out init code from build_all_zonelists
  mm/mm_init.c: Mark mminit_verify_zonelist as __init
  mm/mm_init.c: Mark mminit_loglevel __meminitdata
  kernel/cpuset.c: Mark cpuset_init_current_mems_allowed as __init

 kernel/cpuset.c |  2 +-
 mm/internal.h   |  6 ++++--
 mm/mm_init.c    |  4 ++--
 mm/page_alloc.c | 17 ++++++++++++++---
 4 files changed, 21 insertions(+), 8 deletions(-)

-- 
2.1.3

[PATCH 1/5] mm/internal.h: Don't split printk call in two

[Rasmus Villemoes]

All users of mminit_dprintk pass a compile-time constant as level, so
this just makes gcc emit a single printk call instead of two.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---

Notes:
    Not a huge deal, since the only users are __init or __meminit
    functions, but even there saving 140 bytes may be worthwhile.

 mm/internal.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index efad241f7014..e1d8e05bcfff 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -329,8 +329,10 @@ extern int mminit_loglevel;
 #define mminit_dprintk(level, prefix, fmt, arg...) \
 do { \
 	if (level < mminit_loglevel) { \
-		printk(level <= MMINIT_WARNING ? KERN_WARNING : KERN_DEBUG); \
-		printk(KERN_CONT "mminit::" prefix " " fmt, ##arg); \
+		if (level <= MMINIT_WARNING) \
+			printk(KERN_WARNING "mminit::" prefix " " fmt, ##arg); \
+		else \
+			printk(KERN_DEBUG "mminit::" prefix " " fmt, ##arg); \
 	} \
 } while (0)
 
-- 
2.1.3

[PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists

[Rasmus Villemoes]

Pulling the code protected by if (system_state == SYSTEM_BOOTING) into
its own helper allows us to shrink .text a little. This relies on
build_all_zonelists already having a __ref annotation. Add a comment
explaining why so one doesn't have to track it down through git log.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 mm/page_alloc.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 7633c503a116..c58aa42a3387 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3945,18 +3945,29 @@ static int __build_all_zonelists(void *data)
 	return 0;
 }
 
+static noinline void __init
+build_all_zonelists_init(void)
+{
+	__build_all_zonelists(NULL);
+	mminit_verify_zonelist();
+	cpuset_init_current_mems_allowed();
+}
+
 /*
  * Called with zonelists_mutex held always
  * unless system_state == SYSTEM_BOOTING.
+ *
+ * __ref due to (1) call of __meminit annotated setup_zone_pageset
+ * [we're only called with non-NULL zone through __meminit paths] and
+ * (2) call of __init annotated helper build_all_zonelists_init
+ * [protected by SYSTEM_BOOTING].
  */
 void __ref build_all_zonelists(pg_data_t *pgdat, struct zone *zone)
 {
 	set_zonelist_order();
 
 	if (system_state == SYSTEM_BOOTING) {
-		__build_all_zonelists(NULL);
-		mminit_verify_zonelist();
-		cpuset_init_current_mems_allowed();
+		build_all_zonelists_init();
 	} else {
 #ifdef CONFIG_MEMORY_HOTPLUG
 		if (zone)
-- 
2.1.3

Re: [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists

[David Rientjes]

On Tue, 3 Feb 2015, Rasmus Villemoes wrote:

> Pulling the code protected by if (system_state == SYSTEM_BOOTING) into
> its own helper allows us to shrink .text a little. This relies on
> build_all_zonelists already having a __ref annotation. Add a comment
> explaining why so one doesn't have to track it down through git log.
> 

I think we should see the .text savings in the changelog to decide whether 
we want a __ref function (granted, with comment) calling an __init 
function in the source code.

> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  mm/page_alloc.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 7633c503a116..c58aa42a3387 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3945,18 +3945,29 @@ static int __build_all_zonelists(void *data)
>  	return 0;
>  }
>  
> +static noinline void __init
> +build_all_zonelists_init(void)
> +{
> +	__build_all_zonelists(NULL);
> +	mminit_verify_zonelist();
> +	cpuset_init_current_mems_allowed();
> +}
> +
>  /*
>   * Called with zonelists_mutex held always
>   * unless system_state == SYSTEM_BOOTING.
> + *
> + * __ref due to (1) call of __meminit annotated setup_zone_pageset
> + * [we're only called with non-NULL zone through __meminit paths] and
> + * (2) call of __init annotated helper build_all_zonelists_init
> + * [protected by SYSTEM_BOOTING].
>   */
>  void __ref build_all_zonelists(pg_data_t *pgdat, struct zone *zone)
>  {
>  	set_zonelist_order();
>  
>  	if (system_state == SYSTEM_BOOTING) {
> -		__build_all_zonelists(NULL);
> -		mminit_verify_zonelist();
> -		cpuset_init_current_mems_allowed();
> +		build_all_zonelists_init();
>  	} else {
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  		if (zone)

Re: [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists

[Rasmus Villemoes]

On Tue, Feb 03 2015, David Rientjes <rientjes@google.com> wrote:

> On Tue, 3 Feb 2015, Rasmus Villemoes wrote:
>
>> Pulling the code protected by if (system_state == SYSTEM_BOOTING) into
>> its own helper allows us to shrink .text a little. This relies on
>> build_all_zonelists already having a __ref annotation. Add a comment
>> explaining why so one doesn't have to track it down through git log.
>> 
>
> I think we should see the .text savings in the changelog to decide whether 
> we want a __ref function (granted, with comment) calling an __init 
> function in the source code.

Well, the real saving comes in 3/5, (mm/mm_init.c: Mark
mminit_verify_zonelist as __init), where one saves about 400
bytes. I originally did just that, while still adding a comment to
build_all_zonelists to explain both the old and new cause of __ref.

Then I noticed that cpuset_init_current_mems_allowed is also only called
from build_all_zonelists and could thus also be __init. But then the
__ref would cover two __init functions, both defined elsewhere, so I
thought it would be a little cleaner to make these calls from a single
__init function defined very close to its user. That it also happens to
shave a few bytes from build_all_zonelists is just gravy. A better
commit log would have been something like

  Pulling the code protected by if (system_state == SYSTEM_BOOTING) into
  its own helper allows us to shrink .text by a few bytes. But more
  importantly, this provides a (somewhat) clean way of annotating
  mminit_verify_zonelist and cpuset_init_current_mems_allowed with
  __init, thus saving around 450 bytes of .text.

  This relies on build_all_zonelists already having a __ref
  annotation. Add a comment explaining both uses so one doesn't have to
  track it down through git log.

Rasmus

[PATCH 3/5] mm/mm_init.c: Mark mminit_verify_zonelist as __init

[Rasmus Villemoes]

The only caller of mminit_verify_zonelist is build_all_zonelists_init,
which is annotated with __init, so it should be safe to also mark the
former as __init, saving ~400 bytes of .text.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 mm/mm_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index 4074caf9936b..e17c758b27bf 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -21,7 +21,7 @@ int mminit_loglevel;
 #endif
 
 /* The zonelists are simply reported, validation is manual. */
-void mminit_verify_zonelist(void)
+void __init mminit_verify_zonelist(void)
 {
 	int nid;
 
-- 
2.1.3

[PATCH 4/5] mm/mm_init.c: Mark mminit_loglevel __meminitdata

[Rasmus Villemoes]

mminit_loglevel is only referenced from __init and __meminit
functions, so we can mark it __meminitdata.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 mm/mm_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index e17c758b27bf..5f420f7fafa1 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -14,7 +14,7 @@
 #include "internal.h"
 
 #ifdef CONFIG_DEBUG_MEMORY_INIT
-int mminit_loglevel;
+int __meminitdata mminit_loglevel;
 
 #ifndef SECTIONS_SHIFT
 #define SECTIONS_SHIFT	0
-- 
2.1.3

[PATCH 5/5] kernel/cpuset.c: Mark cpuset_init_current_mems_allowed as __init

[Rasmus Villemoes]

The only caller of cpuset_init_current_mems_allowed is the __init
annotated build_all_zonelists_init, so we can also make the former
__init.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 kernel/cpuset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 64b257f6bca2..c54a1dae6c11 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2400,7 +2400,7 @@ void cpuset_cpus_allowed_fallback(struct task_struct *tsk)
 	 */
 }
 
-void cpuset_init_current_mems_allowed(void)
+void __init cpuset_init_current_mems_allowed(void)
 {
 	nodes_setall(current->mems_allowed);
 }
-- 
2.1.3