Xen project Mailing List

Re: [Xen-devel] [PATCH v2 4/5] alternative/x86/arm32: Align altinstructions (and altinstr_replacement) sections.

From: Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx>

Date: Mon, 11 Sep 2017 14:59:55 -0400

Cc: julien.grall@xxxxxxx, sstabellini@xxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, andrew.cooper3@xxxxxxxxxx

Delivery-date: Tue, 12 Sep 2017 04:48:35 +0000

List-id: Xen developer discussion <xen-devel.lists.xen.org>

On Mon, Jul 31, 2017 at 08:01:18AM -0600, Jan Beulich wrote: > >>> Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx> 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: 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" [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 >

Attachment: diff.gz
Description: application/gzip

Attachment: mm.S.baseline.gz
Description: application/gzip

Attachment: mm.S.new.gz
Description: application/gzip

_______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.