* [PATCH 0/5] mm: Some .text savings
@ 2015-02-02 23:50 Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 1/5] mm/internal.h: Don't split printk call in two Rasmus Villemoes
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2015-02-02 23:50 UTC (permalink / raw)
To: Andrew Morton, Li Zefan; +Cc: Rasmus Villemoes, cgroups, linux-kernel, linux-mm
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/5] mm/internal.h: Don't split printk call in two
2015-02-02 23:50 [PATCH 0/5] mm: Some .text savings Rasmus Villemoes
@ 2015-02-02 23:50 ` Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists Rasmus Villemoes
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2015-02-02 23:50 UTC (permalink / raw)
To: Andrew Morton, Vlastimil Babka, Rik van Riel, Joonsoo Kim
Cc: Rasmus Villemoes, linux-mm, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists
2015-02-02 23:50 [PATCH 0/5] mm: Some .text savings Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 1/5] mm/internal.h: Don't split printk call in two Rasmus Villemoes
@ 2015-02-02 23:50 ` Rasmus Villemoes
2015-02-03 0:25 ` David Rientjes
2015-02-02 23:50 ` [PATCH 3/5] mm/mm_init.c: Mark mminit_verify_zonelist as __init Rasmus Villemoes
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Rasmus Villemoes @ 2015-02-02 23:50 UTC (permalink / raw)
To: Andrew Morton, Vishnu Pratap Singh, Pintu Kumar, Michal Nazarewicz
Cc: Rasmus Villemoes, linux-mm, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/5] mm/mm_init.c: Mark mminit_verify_zonelist as __init
2015-02-02 23:50 [PATCH 0/5] mm: Some .text savings Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 1/5] mm/internal.h: Don't split printk call in two Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists Rasmus Villemoes
@ 2015-02-02 23:50 ` Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 4/5] mm/mm_init.c: Mark mminit_loglevel __meminitdata Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 5/5] kernel/cpuset.c: Mark cpuset_init_current_mems_allowed as __init Rasmus Villemoes
4 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2015-02-02 23:50 UTC (permalink / raw)
To: Andrew Morton, Mel Gorman, Paul Gortmaker, Peter Zijlstra,
Rik van Riel, Tim Chen, Hugh Dickins
Cc: Rasmus Villemoes, linux-mm, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/5] mm/mm_init.c: Mark mminit_loglevel __meminitdata
2015-02-02 23:50 [PATCH 0/5] mm: Some .text savings Rasmus Villemoes
` (2 preceding siblings ...)
2015-02-02 23:50 ` [PATCH 3/5] mm/mm_init.c: Mark mminit_verify_zonelist as __init Rasmus Villemoes
@ 2015-02-02 23:50 ` Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 5/5] kernel/cpuset.c: Mark cpuset_init_current_mems_allowed as __init Rasmus Villemoes
4 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2015-02-02 23:50 UTC (permalink / raw)
To: Andrew Morton, Mel Gorman, Paul Gortmaker, Peter Zijlstra,
Rik van Riel, Tim Chen, Hugh Dickins
Cc: Rasmus Villemoes, linux-mm, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/5] kernel/cpuset.c: Mark cpuset_init_current_mems_allowed as __init
2015-02-02 23:50 [PATCH 0/5] mm: Some .text savings Rasmus Villemoes
` (3 preceding siblings ...)
2015-02-02 23:50 ` [PATCH 4/5] mm/mm_init.c: Mark mminit_loglevel __meminitdata Rasmus Villemoes
@ 2015-02-02 23:50 ` Rasmus Villemoes
4 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2015-02-02 23:50 UTC (permalink / raw)
To: Andrew Morton, Li Zefan; +Cc: Rasmus Villemoes, cgroups, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists
2015-02-02 23:50 ` [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists Rasmus Villemoes
@ 2015-02-03 0:25 ` David Rientjes
2015-02-03 9:57 ` Rasmus Villemoes
0 siblings, 1 reply; 8+ messages in thread
From: David Rientjes @ 2015-02-03 0:25 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Andrew Morton, Vishnu Pratap Singh, Pintu Kumar,
Michal Nazarewicz, linux-mm, linux-kernel
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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists
2015-02-03 0:25 ` David Rientjes
@ 2015-02-03 9:57 ` Rasmus Villemoes
0 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2015-02-03 9:57 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Vishnu Pratap Singh, Pintu Kumar,
Michal Nazarewicz, linux-mm, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-02-03 9:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-02 23:50 [PATCH 0/5] mm: Some .text savings Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 1/5] mm/internal.h: Don't split printk call in two Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 2/5] mm/page_alloc.c: Pull out init code from build_all_zonelists Rasmus Villemoes
2015-02-03 0:25 ` David Rientjes
2015-02-03 9:57 ` Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 3/5] mm/mm_init.c: Mark mminit_verify_zonelist as __init Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 4/5] mm/mm_init.c: Mark mminit_loglevel __meminitdata Rasmus Villemoes
2015-02-02 23:50 ` [PATCH 5/5] kernel/cpuset.c: Mark cpuset_init_current_mems_allowed as __init Rasmus Villemoes
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).