diff mbox

[v2,4/5] alternative/x86/arm32: Align altinstructions (and altinstr_replacement) sections.

Message ID 20170911185947.GA18345@localhost.localdomain (mailing list archive)
State New, archived
Headers show

Commit Message

Konrad Rzeszutek Wilk Sept. 11, 2017, 6:59 p.m. UTC 
On Mon, Jul 31, 2017 at 08:01:18AM -0600, Jan Beulich wrote:
> >>> Konrad Rzeszutek Wilk <konrad@kernel.org> 07/26/17 9:50 PM >>>
> >On x86 the bloat-o-meter detects that with this change the file shrinks:
> >add/remove: 1/0 grow/shrink: 0/2 up/down: 156/-367 (-211)
> >function                                     old     new   delta
> >get_page_from_gfn                              -     156    +156
> >do_mmu_update                               4578    4569      -9
> >do_mmuext_op                                5604    5246    -358
> >Total: Before=3170439, After=3170228, chg -0.01%
> 
> This looks unexpected, and hence I'd like to ask for an explanation. If
> anything I'd expect the image size to grow (slightly).

With just:

[konrad@localhost xen]$ ~/linux/scripts/bloat-o-meter /tmp/xen-syms.baseline.debug xen-syms
add/remove: 0/0 grow/shrink: 1/2 up/down: 87/-237 (-150)
function                                     old     new   delta
sh_page_fault__guest_4                      8557    8644     +87
map_pages_to_xen                            4406    4322     -84
sh_x86_emulate_write__guest_4                444     291    -153
Total: Before=3315124, After=3314974, chg -0.00%


In other words, just adding an useless comment makes it smaller!?


Since this seems to be happening with arch/x86/mm.o, so I compiled the
.S for both the baseline and then one with the "HI!" comment.

Looking at the diff (attached) the difference seems to be that the
compiler optimized some of the operations.

In other words the bloat-o-meter for x86 is useless for this.
> 
> >--- a/xen/include/asm-x86/alternative.h
> >+++ b/xen/include/asm-x86/alternative.h
> >@@ -56,10 +56,12 @@ extern void alternative_instructions(void);
>  >
> >#define ALTERNATIVE_N(newinstr, feature, number)	\
> >".pushsection .altinstructions,\"a\"\n"		\
> >+	".p2align 2\n"					\
> 
> Can't this then be accompanied by dropping the (over-)alignment
> done in xen.lds.S?

Yes.

And also in the arm one.
> 
> >ALTINSTR_ENTRY(feature, number)			\
> >".section .discard,\"a\",@progbits\n"		\
> >DISCARD_ENTRY(number)				\
> >".section .altinstr_replacement, \"ax\"\n"	\
> >+	".p2align 2\n"					\
> 
> This surely isn't needed on x86.

Nope. But I added to be in sync with the other. But it can be removed.
> 
> Jan
>
diff mbox

Patch

diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h
index db4f08e0e7..43d1851f86 100644
--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -56,6 +56,7 @@  extern void alternative_instructions(void);
 
 #define ALTERNATIVE_N(newinstr, feature, number)       \
        ".pushsection .altinstructions,\"a\"\n"         \
+     ".p2align 2\n" \
        ALTINSTR_ENTRY(feature, number)                 \
        ".section .discard,\"a\",@progbits\n"           \
        DISCARD_ENTRY(number)                           \

$ ~/linux/scripts/bloat-o-meter /tmp/xen-syms.baseline.debug xen-syms
add/remove: 0/0 grow/shrink: 1/2 up/down: 87/-237 (-150)
function                                     old     new   delta
sh_page_fault__guest_4                      8557    8644     +87
map_pages_to_xen                            4406    4322     -84
sh_x86_emulate_write__guest_4                444     291    -153
Total: Before=3315124, After=3314974, chg -0.00%
[konrad@localhost xen]$ 

I get the same amount if the '.p2align 2' is in .altinstr_replacement":

diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h
index db4f08e0e7..201683e29e 100644
--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -60,6 +60,7 @@  extern void alternative_instructions(void);
 	".section .discard,\"a\",@progbits\n"		\
 	DISCARD_ENTRY(number)				\
 	".section .altinstr_replacement, \"ax\"\n"	\
+    ".p2align 2\n" \
 	ALTINSTR_REPLACEMENT(newinstr, feature, number)	\
 	".popsection\n"
 
For fun I changed p2align to 1:

diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h
index db4f08e0e7..13a7204e6b 100644
--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -60,6 +60,7 @@  extern void alternative_instructions(void);
        ".section .discard,\"a\",@progbits\n"           \
        DISCARD_ENTRY(number)                           \
        ".section .altinstr_replacement, \"ax\"\n"      \
+    ".p2align 1\n" \
        ALTINSTR_REPLACEMENT(newinstr, feature, number) \
        ".popsection\n"
 
[konrad@localhost xen]$ ~/linux/scripts/bloat-o-meter /tmp/xen-syms.baseline.debug xen-syms
add/remove: 0/0 grow/shrink: 1/2 up/down: 87/-237 (-150)
function                                     old     new   delta
sh_page_fault__guest_4                      8557    8644     +87
map_pages_to_xen                            4406    4322     -84
sh_x86_emulate_write__guest_4                444     291    -153
Total: Before=3315124, After=3314974, chg -0.00%

And then to 0.

--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -60,6 +60,7 @@  extern void alternative_instructions(void);
        ".section .discard,\"a\",@progbits\n"           \
        DISCARD_ENTRY(number)                           \
        ".section .altinstr_replacement, \"ax\"\n"      \
+    ".p2align 0\n" \
        ALTINSTR_REPLACEMENT(newinstr, feature, number) \
        ".popsection\n"
 
[konrad@localhost xen]$ ~/linux/scripts/bloat-o-meter /tmp/xen-syms.baseline.debug xen-syms
add/remove: 0/0 grow/shrink: 1/2 up/down: 87/-237 (-150)
function                                     old     new   delta
sh_page_fault__guest_4                      8557    8644     +87
map_pages_to_xen                            4406    4322     -84
sh_x86_emulate_write__guest_4                444     291    -153
Total: Before=3315124, After=3314974, chg -0.00%


WTF?!

[konrad@localhost xen]$ git diff
diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h
index db4f08e0e7..6c3d59a7df 100644
--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -60,6 +60,7 @@  extern void alternative_instructions(void);
        ".section .discard,\"a\",@progbits\n"           \
        DISCARD_ENTRY(number)                           \
        ".section .altinstr_replacement, \"ax\"\n"      \
+    "# HI!\n" \
        ALTINSTR_REPLACEMENT(newinstr, feature, number) \
        ".popsection\n"