page_alloc.c: inline __rmqueue()

PATCH Superseded Aaron Lu 6 participants 16 messages Oct 9, 2017 – Oct 18, 2017

Subsystems (2)

Memory management Page allocator

lore

Share & tools

lore

Aaron Lu

author

2017-10-09 05:44 UTC #0 lore raw

PATCH 1/1 page_alloc.c: inline __rmqueue()

__rmqueue() is called by rmqueue_bulk() and rmqueue() under zone->lock and that lock can be heavily contended with memory intensive applications.

Since __rmqueue() is a small function, inline it can save us some time. With the will-it-scale/page_fault1/process benchmark, when using nr_cpu processes to stress buddy:

On a 2 sockets Intel-Skylake machine:
base %change head
77342 +6.3% 82203 will-it-scale.per_process_ops

On a 4 sockets Intel-Skylake machine:
base %change head
75746 +4.6% 79248 will-it-scale.per_process_ops

This patch adds inline to __rmqueue().

Signed-off-by Aaron Lu <[email protected]>

---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/page_alloc.c +1 -1

index 0e309ce4a44a..c9605c7ebaf6 100644
@@ -2291,7 +2291,7 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
  * Do the hard work of removing an element from the buddy allocator.
  * Call me with the zone->lock already held.
  */
-static struct page *__rmqueue(struct zone *zone, unsigned int order,
+static inline struct page *__rmqueue(struct zone *zone, unsigned int order,
 				int migratetype)
 {
 	struct page *page;

Signature · 7 lines

2.13.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Anshuman Khandual

Reviewed-by Tested-by on v1 1/1

2017-10-09 07:37 UTC #1 lore raw

Quoted 11 lines · Aaron Lu

__rmqueue() is called by rmqueue_bulk() and rmqueue() under zone->lock and that lock can be heavily contended with memory intensive applications.
Since __rmqueue() is a small function, inline it can save us some time. With the will-it-scale/page_fault1/process benchmark, when using nr_cpu processes to stress buddy:
On a 2 sockets Intel-Skylake machine:
base %change head
77342 +6.3% 82203 will-it-scale.per_process_ops

Quoted 7 lines

On a 4 sockets Intel-Skylake machine:
base %change head
75746 +4.6% 79248 will-it-scale.per_process_ops
This patch adds inline to __rmqueue().
Signed-off-by Aaron Lu <[email protected]>

Ran it through kernel bench and ebizzy micro benchmarks. Results were comparable with and without the patch. May be these are not the appropriate tests for this inlining improvement. Anyways it does not have any performance degradation either.

Reviewed-by Anshuman Khandual <[email protected]>

Tested-by Anshuman Khandual <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Aaron Lu

author

2017-10-09 07:53 UTC #2 lore raw

Quoted 16 lines · Anshuman Khandual

Quoted 15 lines · Aaron Lu
__rmqueue() is called by rmqueue_bulk() and rmqueue() under zone->lock and that lock can be heavily contended with memory intensive applications.
Since __rmqueue() is a small function, inline it can save us some time. With the will-it-scale/page_fault1/process benchmark, when using nr_cpu processes to stress buddy:
On a 2 sockets Intel-Skylake machine:
base %change head
77342 +6.3% 82203 will-it-scale.per_process_ops
On a 4 sockets Intel-Skylake machine:
base %change head
75746 +4.6% 79248 will-it-scale.per_process_ops

Quoted 7 lines

Quoted 3 lines
This patch adds inline to __rmqueue().
Signed-off-by Aaron Lu <[email protected]>
Ran it through kernel bench and ebizzy micro benchmarks. Results were comparable with and without the patch. May be these are not the appropriate tests for this inlining improvement. Anyways it

I think so.

The benefit only appears when the lock contention is huge enough, e.g. perf-profile.self.cycles-pp.native_queued_spin_lock_slowpath is as high as 80% with the workload I have used.

Quoted 4 lines

does not have any performance degradation either.
Reviewed-by Anshuman Khandual <[email protected]>
Tested-by Anshuman Khandual <[email protected]>

Thanks!

Dave Hansen

2017-10-09 20:23 UTC #3 lore raw

In reply to Aaron Lu

Quoted 2 lines · Aaron Lu

__rmqueue() is called by rmqueue_bulk() and rmqueue() under zone->lock and that lock can be heavily contended with memory intensive applications.

What does "memory intensive" mean? I'd probably just say: "The two __rmqueue() call sites are in very hot page allocator paths."

Quoted 3 lines

Since __rmqueue() is a small function, inline it can save us some time. With the will-it-scale/page_fault1/process benchmark, when using nr_cpu processes to stress buddy:

Please include a description of the test and a link to the source.

Quoted 3 lines

On a 2 sockets Intel-Skylake machine:
base %change head
77342 +6.3% 82203 will-it-scale.per_process_ops

What's the unit here? That seems ridiculously low for page_fault1. It's usually in the millions.

Quoted 3 lines

On a 4 sockets Intel-Skylake machine:
base %change head
75746 +4.6% 79248 will-it-scale.per_process_ops

It's probably worth noting the reason that this is _less_ beneficial on a larger system.

I'd also just put this in text rather than wasting space in tables like that. It took me a few minutes to figure out what the table was trying top say. This is one of those places where LKP output is harmful.

Why not just say:

This patch improved the benchmark by 6.3% on a 2-socket system and 4.6% on a 4-socket system.

Quoted 1 line

This patch adds inline to __rmqueue().

How much text bloat does this cost?

Aaron Lu

author

2017-10-10 02:51 UTC #4 lore raw

Quoted 7 lines · Dave Hansen

Quoted 3 lines · Aaron Lu
On a 2 sockets Intel-Skylake machine:
base %change head
77342 +6.3% 82203 will-it-scale.per_process_ops
What's the unit here? That seems ridiculously low for page_fault1. It's usually in the millions.

per_process_ops = processes/nr_process

since nr_process here is nr_cpu, so on the 2 sockets machine with 104 CPUs, processes are 8043568(base) and 8549112(head), which are in the millions as you correctly pointed out.

Aaron Lu

author

2017-10-10 02:56 UTC #5 lore raw

PATCH v2 1/1 mm/page_alloc.c: inline __rmqueue()

__rmqueue() is called by rmqueue_bulk() and rmqueue() under zone->lock and the two __rmqueue() call sites are in very hot page allocator paths.

Since __rmqueue() is a small function, inline it can save us some time. With the will-it-scale/page_fault1/process benchmark, when using nr_cpu processes to stress buddy, this patch improved the benchmark by 6.3% on a 2-sockets Intel-Skylake system and 4.6% on a 4-sockets Intel-Skylake system. The benefit being less on 4 sockets machine is due to the lock contention there(perf-profile/native_queued_spin_lock_slowpath=81%) is less severe than on the 2 sockets machine(84%).

What the benchmark does is: it forks nr_cpu processes and then each process does the following:

    1 mmap() 128M anonymous space;
    2 writes to each page there to trigger actual page allocation;
    3 munmap() it.

in a loop.
https://github.com/antonblanchard/will-it-scale/blob/master/tests/page_fault1.c

This patch adds inline to __rmqueue() and vmlinux' size doesn't have any change after this patch according to size(1).

without this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux

with this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux

Reviewed-by Anshuman Khandual <[email protected]>

Tested-by Anshuman Khandual <[email protected]>

Signed-off-by Aaron Lu <[email protected]>

---
v2: change commit message according to Dave Hansen's suggestion.

 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/page_alloc.c +1 -1

index 0e309ce4a44a..c9605c7ebaf6 100644
@@ -2291,7 +2291,7 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
  * Do the hard work of removing an element from the buddy allocator.
  * Call me with the zone->lock already held.
  */
-static struct page *__rmqueue(struct zone *zone, unsigned int order,
+static inline struct page *__rmqueue(struct zone *zone, unsigned int order,
 				int migratetype)
 {
 	struct page *page;

Signature · 7 lines

2.13.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dave Hansen

2017-10-10 05:19 UTC #6 lore raw

Re: [PATCH v2] mm/page_alloc.c: inline __rmqueue()

Quoted 10 lines · Aaron Lu

This patch adds inline to __rmqueue() and vmlinux' size doesn't have any change after this patch according to size(1).
without this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux
with this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux

This is unexpected. Could you double-check this, please?

Aaron Lu

author

2017-10-10 05:43 UTC #7 lore raw

Re: [PATCH v2] mm/page_alloc.c: inline __rmqueue()

Quoted 8 lines · Dave Hansen

Quoted 7 lines · Aaron Lu
This patch adds inline to __rmqueue() and vmlinux' size doesn't have any change after this patch according to size(1).
without this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux

Quoted 5 lines

Quoted 3 lines
with this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux
This is unexpected. Could you double-check this, please?

mm/page_alloc.o has size changes:

Without this patch:

$ size mm/page_alloc.o
  text    data     bss     dec     hex filename
 36695    9792    8396   54883    d663 mm/page_alloc.o

With this patch:

$ size mm/page_alloc.o
  text    data     bss     dec     hex filename
 37511    9792    8396   55699    d993 mm/page_alloc.o

But vmlinux doesn't.

It's not clear to me what happened, do you want to me dig this out?

Andrew Morton

maintainer

2017-10-10 21:45 UTC #8 lore raw

Re: [PATCH v2] mm/page_alloc.c: inline __rmqueue()

Quoted 22 lines · On Tue, 10 Oct 2017 13:43:43 +0800 Aaron Lu <[email protected]> wrote:

Quoted 8 lines · Dave Hansen
Quoted 7 lines · Aaron Lu
This patch adds inline to __rmqueue() and vmlinux' size doesn't have any change after this patch according to size(1).
without this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux
Quoted 5 lines
Quoted 3 lines
with this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux
This is unexpected. Could you double-check this, please?
mm/page_alloc.o has size changes:
Without this patch:
$ size mm/page_alloc.o
  text    data     bss     dec     hex filename
 36695    9792    8396   54883    d663 mm/page_alloc.o

Quoted 8 lines

With this patch:
$ size mm/page_alloc.o
  text    data     bss     dec     hex filename
 37511    9792    8396   55699    d993 mm/page_alloc.o
But vmlinux doesn't.
It's not clear to me what happened, do you want to me dig this out?

There's weird stuff going on.

With x86_64 gcc-4.8.4

Patch not applied:

akpm3:/usr/local/google/home/akpm/k/25> nm mm/page_alloc.o|grep __rmqueue 0000000000002a00 t __rmqueue

Patch applied:

akpm3:/usr/local/google/home/akpm/k/25> nm mm/page_alloc.o|grep __rmqueue 000000000000039f t __rmqueue_fallback
0000000000001220 t __rmqueue_smallest

So inlining __rmqueue has caused the compiler to decide to uninline __rmqueue_fallback and __rmqueue_smallest, which largely undoes the effect of your patch.

`inline' is basically advisory (or ignored) in modern gcc's. So gcc has felt free to ignore it in __rmqueue_fallback and __rmqueue_smallest because gcc thinks it knows best. That's why we created __always_inline, to grab gcc by the scruff of its neck.

So... I think this patch could do with quite a bit more care, tuning and testing with various gcc versions.

Andi Kleen

2017-10-10 22:27 UTC #9 lore raw

Re: [PATCH v2] mm/page_alloc.c: inline __rmqueue()

Quoted 7 lines

`inline' is basically advisory (or ignored) in modern gcc's. So gcc has felt free to ignore it in __rmqueue_fallback and __rmqueue_smallest because gcc thinks it knows best. That's why we created __always_inline, to grab gcc by the scruff of its neck.
So... I think this patch could do with quite a bit more care, tuning and testing with various gcc versions.

We should just everything in the hot path mark __always_inline.

-Andi

Aaron Lu

author

2017-10-11 02:34 UTC #10 lore raw

Re: [PATCH v2] mm/page_alloc.c: inline __rmqueue()

In reply to Andrew Morton

Quoted 48 lines · Andrew Morton

Quoted 22 lines · On Tue, 10 Oct 2017 13:43:43 +0800 Aaron Lu <[email protected]> wrote:
Quoted 8 lines · Dave Hansen
Quoted 7 lines · Aaron Lu
This patch adds inline to __rmqueue() and vmlinux' size doesn't have any change after this patch according to size(1).
without this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux
Quoted 5 lines
Quoted 3 lines
with this patch:
text data bss dec hex filename
9968576 5793372 17715200 33477148 1fed21c vmlinux
This is unexpected. Could you double-check this, please?
mm/page_alloc.o has size changes:
Without this patch:
$ size mm/page_alloc.o
  text    data     bss     dec     hex filename
 36695    9792    8396   54883    d663 mm/page_alloc.o
Quoted 8 lines
With this patch:
$ size mm/page_alloc.o
  text    data     bss     dec     hex filename
 37511    9792    8396   55699    d993 mm/page_alloc.o
But vmlinux doesn't.
It's not clear to me what happened, do you want to me dig this out?
There's weird stuff going on.
With x86_64 gcc-4.8.4
Patch not applied:
akpm3:/usr/local/google/home/akpm/k/25> nm mm/page_alloc.o|grep __rmqueue 0000000000002a00 t __rmqueue
Patch applied:
akpm3:/usr/local/google/home/akpm/k/25> nm mm/page_alloc.o|grep __rmqueue 000000000000039f t __rmqueue_fallback
0000000000001220 t __rmqueue_smallest

Quoted 8 lines

So inlining __rmqueue has caused the compiler to decide to uninline __rmqueue_fallback and __rmqueue_smallest, which largely undoes the effect of your patch.
`inline' is basically advisory (or ignored) in modern gcc's. So gcc has felt free to ignore it in __rmqueue_fallback and __rmqueue_smallest because gcc thinks it knows best. That's why we created __always_inline, to grab gcc by the scruff of its neck.

This is a good point and I agree with Andi to use always_inline for those functions that we really want to inline.

Quoted 3 lines

So... I think this patch could do with quite a bit more care, tuning and testing with various gcc versions.

I did some more testing.

With x86_64 gcc-4.6.3 available from kernel.org crosstool:

Patch not applied:

[aaron@aaronlu linux]$ nm mm/page_alloc.o |grep __rmqueue 00000000000023f0 t __rmqueue
00000000000027c0 t __rmqueue_pcplist.isra.95

Patch applied:

[aaron@aaronlu linux]$ nm mm/page_alloc.o |grep __rmqueue 0000000000002950 t __rmqueue_pcplist.isra.95

Works expected.

With self built x86_64 gcc-4.8.4:

Patch not applied:

[aaron@aaronlu linux]$ nm mm/page_alloc.o |grep __rmqueue 0000000000001f20 t __rmqueue

Patch applied:

[aaron@aaronlu linux]$ nm mm/page_alloc.o |grep __rmqueue

Works expected.(conflicts with your result though).

I also tested gcc-4.9.4, gcc-5.3.1, gcc-6.4.0 and gcc-7.2.1, all have the same output as the above gcc-4.8.4.

Then I realized CONFIG_OPTIMIZE_INLINING which I always disabled as suggested by the help message(If unsure, say N). Turnining that config on indeed caused gcc-4.8.4 to emit __rmqueue_fallback here.

I think I'll just mark those functions always_inline.

Aaron Lu

author

2017-10-13 06:31 UTC #11 lore raw

PATCH 1/1 mm/page_alloc: make sure __rmqueue() etc. always inline

__rmqueue(), __rmqueue_fallback(), __rmqueue_smallest() and __rmqueue_cma_fallback() are all in page allocator's hot path and better be finished as soon as possible. One way to make them faster is by making them inline. But as Andrew Morton and Andi Kleen pointed out:
https://lkml.org/lkml/2017/10/10/1252
https://lkml.org/lkml/2017/10/10/1279
To make sure they are inlined, we should use __always_inline for them.

With the will-it-scale/page_fault1/process benchmark, when using nr_cpu processes to stress buddy, the results for will-it-scale.processes with and without the patch are:

On a 2-sockets Intel-Skylake machine:

compiler base head

gcc-4.4.7       6496131     6911823 +6.4%
gcc-4.9.4       7225110     7731072 +7.0%
gcc-5.4.1       7054224     7688146 +9.0%
gcc-6.2.0       7059794     7651675 +8.4%

On a 4-sockets Intel-Skylake machine:

compiler base head

gcc-4.4.7      13162890    13508193 +2.6%
gcc-4.9.4      14997463    15484353 +3.2%
gcc-5.4.1      14708711    15449805 +5.0%
gcc-6.2.0      14574099    15349204 +5.3%

The above 4 compilers are used becuase I've done the tests through Intel's Linux Kernel Performance(LKP) infrastructure and they are the available compilers there.

The benefit being less on 4 sockets machine is due to the lock contention there(perf-profile/native_queued_spin_lock_slowpath=81%) is less severe than on the 2 sockets machine(85%).

What the benchmark does is: it forks nr_cpu processes and then each process does the following:

    1 mmap() 128M anonymous space;
    2 writes to each page there to trigger actual page allocation;
    3 munmap() it.

in a loop.
https://github.com/antonblanchard/will-it-scale/blob/master/tests/page_fault1.c

Binary size wise, I have locally built them with different compilers:

[aaron@aaronlu obj]$ size */*/mm/page_alloc.o

   text    data     bss     dec     hex filename
  37409    9904    8524   55837    da1d gcc-4.9.4/base/mm/page_alloc.o
  38273    9904    8524   56701    dd7d gcc-4.9.4/head/mm/page_alloc.o
  37465    9840    8428   55733    d9b5 gcc-5.5.0/base/mm/page_alloc.o
  38169    9840    8428   56437    dc75 gcc-5.5.0/head/mm/page_alloc.o
  37573    9840    8428   55841    da21 gcc-6.4.0/base/mm/page_alloc.o
  38261    9840    8428   56529    dcd1 gcc-6.4.0/head/mm/page_alloc.o
  36863    9840    8428   55131    d75b gcc-7.2.0/base/mm/page_alloc.o
  37711    9840    8428   55979    daab gcc-7.2.0/head/mm/page_alloc.o

Text size increased about 800 bytes for mm/page_alloc.o.

[aaron@aaronlu obj]$ size */*/vmlinux

   text    data     bss     dec       hex     filename
10342757   5903208 17723392 33969357  20654cd gcc-4.9.4/base/vmlinux
10342757   5903208 17723392 33969357  20654cd gcc-4.9.4/head/vmlinux
10332448   5836608 17715200 33884256  2050860 gcc-5.5.0/base/vmlinux
10332448   5836608 17715200 33884256  2050860 gcc-5.5.0/head/vmlinux
10094546   5836696 17715200 33646442  201676a gcc-6.4.0/base/vmlinux
10094546   5836696 17715200 33646442  201676a gcc-6.4.0/head/vmlinux
10018775   5828732 17715200 33562707  2002053 gcc-7.2.0/base/vmlinux
10018775   5828732 17715200 33562707  2002053 gcc-7.2.0/head/vmlinux

Text size for vmlinux has no change though, probably due to function alignment.

Signed-off-by Aaron Lu <[email protected]>

---
 mm/page_alloc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

mm/page_alloc.c +5 -5

index 0e309ce4a44a..0fe3e2095268 100644
@@ -1794,7 +1794,7 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
  * Go through the free lists for the given migratetype and remove
  * the smallest available page from the freelists
  */
-static inline
+static __always_inline
 struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,
 						int migratetype)
 {

@@ -1838,7 +1838,7 @@ static int fallbacks[MIGRATE_TYPES][4] = {
 };
 
 #ifdef CONFIG_CMA
-static struct page *__rmqueue_cma_fallback(struct zone *zone,
+static __always_inline struct page *__rmqueue_cma_fallback(struct zone *zone,
 					unsigned int order)
 {
 	return __rmqueue_smallest(zone, order, MIGRATE_CMA);

@@ -2219,7 +2219,7 @@ static bool unreserve_highatomic_pageblock(const struct alloc_context *ac,
  * deviation from the rest of this file, to make the for loop
  * condition simpler.
  */
-static inline bool
+static __always_inline bool
 __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
 {
 	struct free_area *area;

@@ -2291,8 +2291,8 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
  * Do the hard work of removing an element from the buddy allocator.
  * Call me with the zone->lock already held.
  */
-static struct page *__rmqueue(struct zone *zone, unsigned int order,
-				int migratetype)
+static __always_inline struct page *
+__rmqueue(struct zone *zone, unsigned int order, int migratetype)
 {
 	struct page *page;

Signature · 1 line

2.13.6

Vlastimil Babka

2017-10-17 11:32 UTC #12 lore raw

Re: [PATCH] mm/page_alloc: make sure __rmqueue() etc. always inline

Quoted 37 lines · Aaron Lu

__rmqueue(), __rmqueue_fallback(), __rmqueue_smallest() and __rmqueue_cma_fallback() are all in page allocator's hot path and better be finished as soon as possible. One way to make them faster is by making them inline. But as Andrew Morton and Andi Kleen pointed out:
https://lkml.org/lkml/2017/10/10/1252
https://lkml.org/lkml/2017/10/10/1279
To make sure they are inlined, we should use __always_inline for them.
With the will-it-scale/page_fault1/process benchmark, when using nr_cpu processes to stress buddy, the results for will-it-scale.processes with and without the patch are:
On a 2-sockets Intel-Skylake machine:
compiler base head
gcc-4.4.7       6496131     6911823 +6.4%
gcc-4.9.4       7225110     7731072 +7.0%
gcc-5.4.1       7054224     7688146 +9.0%
gcc-6.2.0       7059794     7651675 +8.4%
On a 4-sockets Intel-Skylake machine:
compiler base head
gcc-4.4.7      13162890    13508193 +2.6%
gcc-4.9.4      14997463    15484353 +3.2%
gcc-5.4.1      14708711    15449805 +5.0%
gcc-6.2.0      14574099    15349204 +5.3%
The above 4 compilers are used becuase I've done the tests through Intel's Linux Kernel Performance(LKP) infrastructure and they are the available compilers there.
The benefit being less on 4 sockets machine is due to the lock contention there(perf-profile/native_queued_spin_lock_slowpath=81%) is less severe than on the 2 sockets machine(85%).

Quoted 7 lines

What the benchmark does is: it forks nr_cpu processes and then each process does the following:
    1 mmap() 128M anonymous space;
    2 writes to each page there to trigger actual page allocation;
    3 munmap() it.
in a loop.
https://github.com/antonblanchard/will-it-scale/blob/master/tests/page_fault1.c

Are transparent hugepages enabled? If yes, __rmqueue() is called from rmqueue(), and there's only one page fault (and __rmqueue()) per 512 "writes to each page". If not, __rmqueue() is called from rmqueue_bulk() in bursts once pcplists are depleted. I guess it's the latter, otherwise I wouldn't expect a function call to have such visible overhead.

I guess what would help much more would be a bulk __rmqueue_smallest() to grab multiple pages from the freelists. But can't argue with your numbers against this patch.

Quoted 13 lines

Binary size wise, I have locally built them with different compilers:

[aaron@aaronlu obj]$ size */*/mm/page_alloc.o

   text    data     bss     dec     hex filename
  37409    9904    8524   55837    da1d gcc-4.9.4/base/mm/page_alloc.o
  38273    9904    8524   56701    dd7d gcc-4.9.4/head/mm/page_alloc.o
  37465    9840    8428   55733    d9b5 gcc-5.5.0/base/mm/page_alloc.o
  38169    9840    8428   56437    dc75 gcc-5.5.0/head/mm/page_alloc.o
  37573    9840    8428   55841    da21 gcc-6.4.0/base/mm/page_alloc.o
  38261    9840    8428   56529    dcd1 gcc-6.4.0/head/mm/page_alloc.o
  36863    9840    8428   55131    d75b gcc-7.2.0/base/mm/page_alloc.o
  37711    9840    8428   55979    daab gcc-7.2.0/head/mm/page_alloc.o

Quoted 1 line

Text size increased about 800 bytes for mm/page_alloc.o.

BTW, do you know about ./scripts/bloat-o-meter? :) With gcc 7.2.1:

Quoted 1 line

./scripts/bloat-o-meter base.o mm/page_alloc.o

add/remove: 1/2 grow/shrink: 2/0 up/down: 2493/-1649 (844)
function old new delta
get_page_from_freelist 2898 4937 +2039
steal_suitable_fallback - 365 +365
find_suitable_fallback 31 120 +89
find_suitable_fallback.part 115 - -115
__rmqueue 1534 - -1534

Quoted 11 lines

[aaron@aaronlu obj]$ size */*/vmlinux

   text    data     bss     dec       hex     filename
10342757   5903208 17723392 33969357  20654cd gcc-4.9.4/base/vmlinux
10342757   5903208 17723392 33969357  20654cd gcc-4.9.4/head/vmlinux
10332448   5836608 17715200 33884256  2050860 gcc-5.5.0/base/vmlinux
10332448   5836608 17715200 33884256  2050860 gcc-5.5.0/head/vmlinux
10094546   5836696 17715200 33646442  201676a gcc-6.4.0/base/vmlinux
10094546   5836696 17715200 33646442  201676a gcc-6.4.0/head/vmlinux
10018775   5828732 17715200 33562707  2002053 gcc-7.2.0/base/vmlinux
10018775   5828732 17715200 33562707  2002053 gcc-7.2.0/head/vmlinux

Quoted 2 lines

Text size for vmlinux has no change though, probably due to function alignment.

Yep that's useless to show. These differences do add up though, until they eventually cross the alignment boundary.

Thanks,
Vlastimil

Quoted 49 lines

Signed-off-by Aaron Lu <[email protected]>

---
 mm/page_alloc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

mm/page_alloc.c +5 -5

index 0e309ce4a44a..0fe3e2095268 100644
@@ -1794,7 +1794,7 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
  * Go through the free lists for the given migratetype and remove
  * the smallest available page from the freelists
  */
-static inline
+static __always_inline
 struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,
 						int migratetype)
 {

@@ -1838,7 +1838,7 @@ static int fallbacks[MIGRATE_TYPES][4] = {
 };
 
 #ifdef CONFIG_CMA
-static struct page *__rmqueue_cma_fallback(struct zone *zone,
+static __always_inline struct page *__rmqueue_cma_fallback(struct zone *zone,
 					unsigned int order)
 {
 	return __rmqueue_smallest(zone, order, MIGRATE_CMA);

@@ -2219,7 +2219,7 @@ static bool unreserve_highatomic_pageblock(const struct alloc_context *ac,
  * deviation from the rest of this file, to make the for loop
  * condition simpler.
  */
-static inline bool
+static __always_inline bool
 __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
 {
 	struct free_area *area;

@@ -2291,8 +2291,8 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
  * Do the hard work of removing an element from the buddy allocator.
  * Call me with the zone->lock already held.
  */
-static struct page *__rmqueue(struct zone *zone, unsigned int order,
-				int migratetype)
+static __always_inline struct page *
+__rmqueue(struct zone *zone, unsigned int order, int migratetype)
 {
 	struct page *page;

Lu, Aaron

author

2017-10-18 01:54 UTC #13 lore raw

Re: [PATCH] mm/page_alloc: make sure __rmqueue() etc. always inline

T24gVHVlLCAyMDE3LTEwLTE3IGF0IDEzOjMyICswMjAwLCBWbGFzdGltaWwgQmFia2Egd3JvdGU6 DQo+IE9uIDEwLzEzLzIwMTcgMDg6MzEgQU0sIEFhcm9uIEx1IHdyb3RlOg0KPiA+IF9fcm1xdWV1 ZSgpLCBfX3JtcXVldWVfZmFsbGJhY2soKSwgX19ybXF1ZXVlX3NtYWxsZXN0KCkgYW5kDQo+ID4g X19ybXF1ZXVlX2NtYV9mYWxsYmFjaygpIGFyZSBhbGwgaW4gcGFnZSBhbGxvY2F0b3IncyBob3Qg cGF0aCBhbmQNCj4gPiBiZXR0ZXIgYmUgZmluaXNoZWQgYXMgc29vbiBhcyBwb3NzaWJsZS4gT25l IHdheSB0byBtYWtlIHRoZW0gZmFzdGVyDQo+ID4gaXMgYnkgbWFraW5nIHRoZW0gaW5saW5lLiBC dXQgYXMgQW5kcmV3IE1vcnRvbiBhbmQgQW5kaSBLbGVlbiBwb2ludGVkDQo+ID4gb3V0Og0KPiA+ IGh0dHBzOi8vbGttbC5vcmcvbGttbC8yMDE3LzEwLzEwLzEyNTINCj4gPiBodHRwczovL2xrbWwu b3JnL2xrbWwvMjAxNy8xMC8xMC8xMjc5DQo+ID4gVG8gbWFrZSBzdXJlIHRoZXkgYXJlIGlubGlu ZWQsIHdlIHNob3VsZCB1c2UgX19hbHdheXNfaW5saW5lIGZvciB0aGVtLg0KPiA+IA0KPiA+IFdp dGggdGhlIHdpbGwtaXQtc2NhbGUvcGFnZV9mYXVsdDEvcHJvY2VzcyBiZW5jaG1hcmssIHdoZW4g dXNpbmcgbnJfY3B1DQo+ID4gcHJvY2Vzc2VzIHRvIHN0cmVzcyBidWRkeSwgdGhlIHJlc3VsdHMg Zm9yIHdpbGwtaXQtc2NhbGUucHJvY2Vzc2VzIHdpdGgNCj4gPiBhbmQgd2l0aG91dCB0aGUgcGF0 Y2ggYXJlOg0KPiA+IA0KPiA+IE9uIGEgMi1zb2NrZXRzIEludGVsLVNreWxha2UgbWFjaGluZToN Cj4gPiANCj4gPiAgY29tcGlsZXIgICAgICAgICAgYmFzZSAgICAgICAgaGVhZA0KPiA+IGdjYy00 LjQuNyAgICAgICA2NDk2MTMxICAgICA2OTExODIzICs2LjQlDQo+ID4gZ2NjLTQuOS40ICAgICAg IDcyMjUxMTAgICAgIDc3MzEwNzIgKzcuMCUNCj4gPiBnY2MtNS40LjEgICAgICAgNzA1NDIyNCAg ICAgNzY4ODE0NiArOS4wJQ0KPiA+IGdjYy02LjIuMCAgICAgICA3MDU5Nzk0ICAgICA3NjUxNjc1 ICs4LjQlDQo+ID4gDQo+ID4gT24gYSA0LXNvY2tldHMgSW50ZWwtU2t5bGFrZSBtYWNoaW5lOg0K PiA+IA0KPiA+ICBjb21waWxlciAgICAgICAgICBiYXNlICAgICAgICBoZWFkDQo+ID4gZ2NjLTQu NC43ICAgICAgMTMxNjI4OTAgICAgMTM1MDgxOTMgKzIuNiUNCj4gPiBnY2MtNC45LjQgICAgICAx NDk5NzQ2MyAgICAxNTQ4NDM1MyArMy4yJQ0KPiA+IGdjYy01LjQuMSAgICAgIDE0NzA4NzExICAg IDE1NDQ5ODA1ICs1LjAlDQo+ID4gZ2NjLTYuMi4wICAgICAgMTQ1NzQwOTkgICAgMTUzNDkyMDQg KzUuMyUNCj4gPiANCj4gPiBUaGUgYWJvdmUgNCBjb21waWxlcnMgYXJlIHVzZWQgYmVjdWFzZSBJ J3ZlIGRvbmUgdGhlIHRlc3RzIHRocm91Z2ggSW50ZWwncw0KPiA+IExpbnV4IEtlcm5lbCBQZXJm b3JtYW5jZShMS1ApIGluZnJhc3RydWN0dXJlIGFuZCB0aGV5IGFyZSB0aGUgYXZhaWxhYmxlDQo+ ID4gY29tcGlsZXJzIHRoZXJlLg0KPiA+IA0KPiA+IFRoZSBiZW5lZml0IGJlaW5nIGxlc3Mgb24g NCBzb2NrZXRzIG1hY2hpbmUgaXMgZHVlIHRvIHRoZSBsb2NrIGNvbnRlbnRpb24NCj4gPiB0aGVy ZShwZXJmLXByb2ZpbGUvbmF0aXZlX3F1ZXVlZF9zcGluX2xvY2tfc2xvd3BhdGg9ODElKSBpcyBs ZXNzIHNldmVyZQ0KPiA+IHRoYW4gb24gdGhlIDIgc29ja2V0cyBtYWNoaW5lKDg1JSkuDQo+ID4g DQo+ID4gV2hhdCB0aGUgYmVuY2htYXJrIGRvZXMgaXM6IGl0IGZvcmtzIG5yX2NwdSBwcm9jZXNz ZXMgYW5kIHRoZW4gZWFjaA0KPiA+IHByb2Nlc3MgZG9lcyB0aGUgZm9sbG93aW5nOg0KPiA+ICAg ICAxIG1tYXAoKSAxMjhNIGFub255bW91cyBzcGFjZTsNCj4gPiAgICAgMiB3cml0ZXMgdG8gZWFj aCBwYWdlIHRoZXJlIHRvIHRyaWdnZXIgYWN0dWFsIHBhZ2UgYWxsb2NhdGlvbjsNCj4gPiAgICAg MyBtdW5tYXAoKSBpdC4NCj4gPiBpbiBhIGxvb3AuDQo+ID4gaHR0cHM6Ly9naXRodWIuY29tL2Fu dG9uYmxhbmNoYXJkL3dpbGwtaXQtc2NhbGUvYmxvYi9tYXN0ZXIvdGVzdHMvcGFnZV9mYXVsdDEu Yw0KPiANCj4gQXJlIHRyYW5zcGFyZW50IGh1Z2VwYWdlcyBlbmFibGVkPyBJZiB5ZXMsIF9fcm1x dWV1ZSgpIGlzIGNhbGxlZCBmcm9tDQo+IHJtcXVldWUoKSwgYW5kIHRoZXJlJ3Mgb25seSBvbmUg cGFnZSBmYXVsdCAoYW5kIF9fcm1xdWV1ZSgpKSBwZXIgNTEyDQo+ICJ3cml0ZXMgdG8gZWFjaCBw YWdlIi4gSWYgbm90LCBfX3JtcXVldWUoKSBpcyBjYWxsZWQgZnJvbSBybXF1ZXVlX2J1bGsoKQ0K PiBpbiBidXJzdHMgb25jZSBwY3BsaXN0cyBhcmUgZGVwbGV0ZWQuIEkgZ3Vlc3MgaXQncyB0aGUg bGF0dGVyLCBvdGhlcndpc2UNCj4gSSB3b3VsZG4ndCBleHBlY3QgYSBmdW5jdGlvbiBjYWxsIHRv IGhhdmUgc3VjaCB2aXNpYmxlIG92ZXJoZWFkLg0KDQpUSFAgaXMgZGlzYWJsZWQuIEkgc2hvdWxk IGhhdmUgbWVudGlvbmVkIHRoaXMgaW4gdGhlIGNoYW5nZWxvZywgc29ycnkNCmFib3V0IHRoYXQu DQoNCj4gDQo+IEkgZ3Vlc3Mgd2hhdCB3b3VsZCBoZWxwIG11Y2ggbW9yZSB3b3VsZCBiZSBhIGJ1 bGsgX19ybXF1ZXVlX3NtYWxsZXN0KCkNCj4gdG8gZ3JhYiBtdWx0aXBsZSBwYWdlcyBmcm9tIHRo ZSBmcmVlbGlzdHMuIEJ1dCBjYW4ndCBhcmd1ZSB3aXRoIHlvdXINCg0KRG8gSSB1bmRlcnN0YW5k IHlvdSBjb3JyZWN0bHkgdGhhdCB5b3Ugc3VnZ2VzdCB0byB1c2UgYSBidWxrDQpfX3JtcXVldWVf c21hbGxlc3QoKSwgc2F5IF9fcm1xdWV1ZV9zbWFsbGVzdF9idWxrKCkuIFdpdGggdGhhdCwgaW5z dGVhZA0Kb2YgbG9vcGluZyBwY3AtPmJhdGNoIHRpbWVzIGluIHJtcXVldWVfYnVsaygpLCBhIHNp bmdsZSBjYWxsIHRvDQpfX3JtcXVldWVfc21hbGxlc3RfYnVsaygpIGlzIGVub3VnaCBhbmQgX19y bXF1ZXVlX3NtYWxsZXN0X2J1bGsoKSB3aWxsDQpsb29wIHBjcC0+YmF0Y2ggdGltZXMgdG8gZ2V0 IHRob3NlIHBhZ2VzPw0KDQpUaGVuIGl0IGZlZWxzIGxpa2UgX19ybXF1ZXVlX3NtYWxsZXN0X2J1 bGsoKSBoYXMgYmVjb21lIHJtcXVldWVfYnVsaygpLA0Kb3IgZG8gSSBtaXNzIHNvbWV0aGluZz8N Cg0KPiBudW1iZXJzIGFnYWluc3QgdGhpcyBwYXRjaC4NCj4gDQo+ID4gQmluYXJ5IHNpemUgd2lz ZSwgSSBoYXZlIGxvY2FsbHkgYnVpbHQgdGhlbSB3aXRoIGRpZmZlcmVudCBjb21waWxlcnM6DQo+ ID4gDQo+ID4gW2Fhcm9uQGFhcm9ubHUgb2JqXSQgc2l6ZSAqLyovbW0vcGFnZV9hbGxvYy5vDQo+ ID4gICAgdGV4dCAgICBkYXRhICAgICBic3MgICAgIGRlYyAgICAgaGV4IGZpbGVuYW1lDQo+ID4g ICAzNzQwOSAgICA5OTA0ICAgIDg1MjQgICA1NTgzNyAgICBkYTFkIGdjYy00LjkuNC9iYXNlL21t L3BhZ2VfYWxsb2Mubw0KPiA+ICAgMzgyNzMgICAgOTkwNCAgICA4NTI0ICAgNTY3MDEgICAgZGQ3 ZCBnY2MtNC45LjQvaGVhZC9tbS9wYWdlX2FsbG9jLm8NCj4gPiAgIDM3NDY1ICAgIDk4NDAgICAg ODQyOCAgIDU1NzMzICAgIGQ5YjUgZ2NjLTUuNS4wL2Jhc2UvbW0vcGFnZV9hbGxvYy5vDQo+ID4g ICAzODE2OSAgICA5ODQwICAgIDg0MjggICA1NjQzNyAgICBkYzc1IGdjYy01LjUuMC9oZWFkL21t L3BhZ2VfYWxsb2Mubw0KPiA+ICAgMzc1NzMgICAgOTg0MCAgICA4NDI4ICAgNTU4NDEgICAgZGEy MSBnY2MtNi40LjAvYmFzZS9tbS9wYWdlX2FsbG9jLm8NCj4gPiAgIDM4MjYxICAgIDk4NDAgICAg ODQyOCAgIDU2NTI5ICAgIGRjZDEgZ2NjLTYuNC4wL2hlYWQvbW0vcGFnZV9hbGxvYy5vDQo+ID4g ICAzNjg2MyAgICA5ODQwICAgIDg0MjggICA1NTEzMSAgICBkNzViIGdjYy03LjIuMC9iYXNlL21t L3BhZ2VfYWxsb2Mubw0KPiA+ICAgMzc3MTEgICAgOTg0MCAgICA4NDI4ICAgNTU5NzkgICAgZGFh YiBnY2MtNy4yLjAvaGVhZC9tbS9wYWdlX2FsbG9jLm8NCj4gPiANCj4gPiBUZXh0IHNpemUgaW5j cmVhc2VkIGFib3V0IDgwMCBieXRlcyBmb3IgbW0vcGFnZV9hbGxvYy5vLg0KPiANCj4gQlRXLCBk byB5b3Uga25vdyBhYm91dCAuL3NjcmlwdHMvYmxvYXQtby1tZXRlcj8gOikNCg0KTk8hISEgVGhh bmtzIGZvciBicmluZ2luZyB0aGlzIHVwIDopDQoNCj4gV2l0aCBnY2MgNy4yLjE6DQo+ID4gLi9z Y3JpcHRzL2Jsb2F0LW8tbWV0ZXIgYmFzZS5vIG1tL3BhZ2VfYWxsb2Mubw0KPiANCj4gYWRkL3Jl bW92ZTogMS8yIGdyb3cvc2hyaW5rOiAyLzAgdXAvZG93bjogMjQ5My8tMTY0OSAoODQ0KQ0KDQpO aWNlLCBpdCBjbGVhcmx5IHNob3dlZCA4NDQgYnl0ZXMgYmxvYXQuDQoNCj4gZnVuY3Rpb24gICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgb2xkICAgICBuZXcgICBkZWx0YQ0KPiBn ZXRfcGFnZV9mcm9tX2ZyZWVsaXN0ICAgICAgICAgICAgICAgICAgICAgIDI4OTggICAgNDkzNyAg ICsyMDM5DQo+IHN0ZWFsX3N1aXRhYmxlX2ZhbGxiYWNrICAgICAgICAgICAgICAgICAgICAgICAg LSAgICAgMzY1ICAgICszNjUNCj4gZmluZF9zdWl0YWJsZV9mYWxsYmFjayAgICAgICAgICAgICAg ICAgICAgICAgIDMxICAgICAxMjAgICAgICs4OQ0KPiBmaW5kX3N1aXRhYmxlX2ZhbGxiYWNrLnBh cnQgICAgICAgICAgICAgICAgICAxMTUgICAgICAgLSAgICAtMTE1DQo+IF9fcm1xdWV1ZSAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgMTUzNCAgICAgICAtICAgLTE1MzQNCj4gDQo+ IA0KPiA+IFthYXJvbkBhYXJvbmx1IG9ial0kIHNpemUgKi8qL3ZtbGludXgNCj4gPiAgICB0ZXh0 ICAgIGRhdGEgICAgIGJzcyAgICAgZGVjICAgICAgIGhleCAgICAgZmlsZW5hbWUNCj4gPiAxMDM0 Mjc1NyAgIDU5MDMyMDggMTc3MjMzOTIgMzM5NjkzNTcgIDIwNjU0Y2QgZ2NjLTQuOS40L2Jhc2Uv dm1saW51eA0KPiA+IDEwMzQyNzU3ICAgNTkwMzIwOCAxNzcyMzM5MiAzMzk2OTM1NyAgMjA2NTRj ZCBnY2MtNC45LjQvaGVhZC92bWxpbnV4DQo+ID4gMTAzMzI0NDggICA1ODM2NjA4IDE3NzE1MjAw IDMzODg0MjU2ICAyMDUwODYwIGdjYy01LjUuMC9iYXNlL3ZtbGludXgNCj4gPiAxMDMzMjQ0OCAg IDU4MzY2MDggMTc3MTUyMDAgMzM4ODQyNTYgIDIwNTA4NjAgZ2NjLTUuNS4wL2hlYWQvdm1saW51 eA0KPiA+IDEwMDk0NTQ2ICAgNTgzNjY5NiAxNzcxNTIwMCAzMzY0NjQ0MiAgMjAxNjc2YSBnY2Mt Ni40LjAvYmFzZS92bWxpbnV4DQo+ID4gMTAwOTQ1NDYgICA1ODM2Njk2IDE3NzE1MjAwIDMzNjQ2 NDQyICAyMDE2NzZhIGdjYy02LjQuMC9oZWFkL3ZtbGludXgNCj4gPiAxMDAxODc3NSAgIDU4Mjg3 MzIgMTc3MTUyMDAgMzM1NjI3MDcgIDIwMDIwNTMgZ2NjLTcuMi4wL2Jhc2Uvdm1saW51eA0KPiA+ IDEwMDE4Nzc1ICAgNTgyODczMiAxNzcxNTIwMCAzMzU2MjcwNyAgMjAwMjA1MyBnY2MtNy4yLjAv aGVhZC92bWxpbnV4DQo+ID4gDQo+ID4gVGV4dCBzaXplIGZvciB2bWxpbnV4IGhhcyBubyBjaGFu Z2UgdGhvdWdoLCBwcm9iYWJseSBkdWUgdG8gZnVuY3Rpb24NCj4gPiBhbGlnbm1lbnQuDQo+IA0K PiBZZXAgdGhhdCdzIHVzZWxlc3MgdG8gc2hvdy4gVGhlc2UgZGlmZmVyZW5jZXMgZG8gYWRkIHVw IHRob3VnaCwgdW50aWwNCj4gdGhleSBldmVudHVhbGx5IGNyb3NzIHRoZSBhbGlnbm1lbnQgYm91 bmRhcnkuDQoNCkFncmVlZC4NCkJ1dCB5b3Uga25vdywgaXQgaXMgdGhlIGhvdCBwYXRoLCB0aGUg cGVyZm9ybWFuY2UgaW1wcm92ZW1lbnQgbWlnaHQgYmUNCndvcnRoIGl0Lg==

Vlastimil Babka

Acked-by

2017-10-18 06:28 UTC #14 lore raw

Re: [PATCH] mm/page_alloc: make sure __rmqueue() etc. always inline

Quoted 10 lines · Aaron

Quoted 6 lines · Vlastimil Babka
Are transparent hugepages enabled? If yes, __rmqueue() is called from rmqueue(), and there's only one page fault (and __rmqueue()) per 512 "writes to each page". If not, __rmqueue() is called from rmqueue_bulk() in bursts once pcplists are depleted. I guess it's the latter, otherwise I wouldn't expect a function call to have such visible overhead.
THP is disabled. I should have mentioned this in the changelog, sorry about that.

OK, then it makes sense!

Quoted 9 lines

Quoted 3 lines
I guess what would help much more would be a bulk __rmqueue_smallest() to grab multiple pages from the freelists. But can't argue with your
Do I understand you correctly that you suggest to use a bulk __rmqueue_smallest(), say __rmqueue_smallest_bulk(). With that, instead of looping pcp->batch times in rmqueue_bulk(), a single call to __rmqueue_smallest_bulk() is enough and __rmqueue_smallest_bulk() will loop pcp->batch times to get those pages?

Yeah, but I looked at it more closely, and maybe there's not much to gain after all. E.g., there seem to be no atomic counter updates that would benefit from batching, or expensive setup/cleanup in __rmqueue_smallest().

Quoted 2 lines

Then it feels like __rmqueue_smallest_bulk() has become rmqueue_bulk(), or do I miss something?

Right, looks like thanks to inlining, the compiler can already achieve most of the potential gains.

Quoted 13 lines

Quoted 4 lines
With gcc 7.2.1:
Quoted 1 line
./scripts/bloat-o-meter base.o mm/page_alloc.o
add/remove: 1/2 grow/shrink: 2/0 up/down: 2493/-1649 (844)
Nice, it clearly showed 844 bytes bloat.
Quoted 6 lines
function old new delta
get_page_from_freelist 2898 4937 +2039
steal_suitable_fallback - 365 +365
find_suitable_fallback 31 120 +89
find_suitable_fallback.part 115 - -115
__rmqueue 1534 - -1534

It also shows that steal_suitable_fallback() is no longer inlined. Which is fine, because that should ideally be rarely executed.

Quoted 15 lines

Quoted 14 lines

[aaron@aaronlu obj]$ size */*/vmlinux

   text    data     bss     dec       hex     filename
10342757   5903208 17723392 33969357  20654cd gcc-4.9.4/base/vmlinux
10342757   5903208 17723392 33969357  20654cd gcc-4.9.4/head/vmlinux
10332448   5836608 17715200 33884256  2050860 gcc-5.5.0/base/vmlinux
10332448   5836608 17715200 33884256  2050860 gcc-5.5.0/head/vmlinux
10094546   5836696 17715200 33646442  201676a gcc-6.4.0/base/vmlinux
10094546   5836696 17715200 33646442  201676a gcc-6.4.0/head/vmlinux
10018775   5828732 17715200 33562707  2002053 gcc-7.2.0/base/vmlinux
10018775   5828732 17715200 33562707  2002053 gcc-7.2.0/head/vmlinux

Text size for vmlinux has no change though, probably due to function alignment.

Quoted 6 lines

Quoted 2 lines
Yep that's useless to show. These differences do add up though, until they eventually cross the alignment boundary.
Agreed.
But you know, it is the hot path, the performance improvement might be worth it.

I'd agree, so you can add

Acked-by Vlastimil Babka <[email protected]>

Aaron Lu

author

2017-10-18 08:57 UTC #15 lore raw

Re: [PATCH] mm/page_alloc: make sure __rmqueue() etc. always inline

Quoted 16 lines · Vlastimil Babka

Quoted 14 lines · Aaron
Quoted 4 lines · Vlastimil Babka
With gcc 7.2.1:
Quoted 1 line
./scripts/bloat-o-meter base.o mm/page_alloc.o
add/remove: 1/2 grow/shrink: 2/0 up/down: 2493/-1649 (844)
Nice, it clearly showed 844 bytes bloat.
Quoted 6 lines
function old new delta
get_page_from_freelist 2898 4937 +2039
steal_suitable_fallback - 365 +365
find_suitable_fallback 31 120 +89
find_suitable_fallback.part 115 - -115
__rmqueue 1534 - -1534

Quoted 2 lines

It also shows that steal_suitable_fallback() is no longer inlined. Which is fine, because that should ideally be rarely executed.

Ah right, so this script is really good for analysing inline changes.

Quoted 19 lines

Quoted 19 lines
Quoted 18 lines
Quoted 13 lines
[aaron@aaronlu obj]$ size */*/vmlinux
   text    data     bss     dec       hex     filename
10342757   5903208 17723392 33969357  20654cd gcc-4.9.4/base/vmlinux
10342757   5903208 17723392 33969357  20654cd gcc-4.9.4/head/vmlinux
10332448   5836608 17715200 33884256  2050860 gcc-5.5.0/base/vmlinux
10332448   5836608 17715200 33884256  2050860 gcc-5.5.0/head/vmlinux
10094546   5836696 17715200 33646442  201676a gcc-6.4.0/base/vmlinux
10094546   5836696 17715200 33646442  201676a gcc-6.4.0/head/vmlinux
10018775   5828732 17715200 33562707  2002053 gcc-7.2.0/base/vmlinux
10018775   5828732 17715200 33562707  2002053 gcc-7.2.0/head/vmlinux
Quoted 2 lines
Text size for vmlinux has no change though, probably due to function alignment.
Yep that's useless to show. These differences do add up though, until they eventually cross the alignment boundary.

Quoted 7 lines

Quoted 3 lines
Agreed.
But you know, it is the hot path, the performance improvement might be worth it.
I'd agree, so you can add
Acked-by Vlastimil Babka <[email protected]>

Thanks!