| Message ID | 1455293208-6763-2-git-send-email-jeremy.linton@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12 February 2016 at 17:21, Jeremy Linton <jeremy.linton@arm.com> wrote: > On 02/12/2016 10:11 AM, Ard Biesheuvel wrote: >> >> On 12 February 2016 at 17:06, Jeremy Linton <jeremy.linton@arm.com> wrote: > > (trimming) >>> >>> #if defined(CONFIG_DEBUG_ALIGN_RODATA) >>> -#define ALIGN_DEBUG_RO . = ALIGN(1<<SECTION_SHIFT); >>> -#define ALIGN_DEBUG_RO_MIN(min) ALIGN_DEBUG_RO >>> +#if defined(CONFIG_ARM64_64K_PAGES) >>> +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(CONT_SIZE); >>> +#else >>> +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(SECTION_SIZE); >> >> >> Doesn't this align to 32 MB on 16k pages kernels? > > > Yes, I considered whether it was more appropriate to use CONT_SIZE for 16k > as well. > > Opinions? > Looking at vmlinux.lds.S, I see that that would put _stext and __init_begin at 32 MB aligned boundaries. making the size of the kernel at least 64 MB. If I take your .rodata patch into account, which adds a third instance of ALIGN_DEBUG_RO_MIN, the Image footprint will rise to ~100 MB. Or am I missing something? > >> >>> +#endif >>> #elif defined(CONFIG_DEBUG_RODATA) >>> -#define ALIGN_DEBUG_RO . = ALIGN(1<<PAGE_SHIFT); >>> -#define ALIGN_DEBUG_RO_MIN(min) ALIGN_DEBUG_RO >>> +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(PAGE_SIZE); >>> #else >>> -#define ALIGN_DEBUG_RO >>> #define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(min); >>> #endif >>> >>> -- >>> 2.4.3 >>> >> >
On 02/12/2016 10:28 AM, Ard Biesheuvel wrote: > On 12 February 2016 at 17:21, Jeremy Linton <jeremy.linton@arm.com> wrote: >> On 02/12/2016 10:11 AM, Ard Biesheuvel wrote: >>> >>> On 12 February 2016 at 17:06, Jeremy Linton <jeremy.linton@arm.com> wrote: >> >> (trimming) >>>> >>>> #if defined(CONFIG_DEBUG_ALIGN_RODATA) >>>> -#define ALIGN_DEBUG_RO . = ALIGN(1<<SECTION_SHIFT); >>>> -#define ALIGN_DEBUG_RO_MIN(min) ALIGN_DEBUG_RO >>>> +#if defined(CONFIG_ARM64_64K_PAGES) >>>> +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(CONT_SIZE); >>>> +#else >>>> +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(SECTION_SIZE); >>> >>> >>> Doesn't this align to 32 MB on 16k pages kernels? >> >> >> Yes, I considered whether it was more appropriate to use CONT_SIZE for 16k >> as well. >> >> Opinions? >> > > Looking at vmlinux.lds.S, I see that that would put _stext and > __init_begin at 32 MB aligned boundaries. making the size of the > kernel at least 64 MB. If I take your .rodata patch into account, > which adds a third instance of ALIGN_DEBUG_RO_MIN, the Image footprint > will rise to ~100 MB. Or am I missing something? > No, I think your correct. But, its an option, and it sort of depends on use case. In a system with 100+GB of RAM it might be useful. Not so much on a phone or small embedded system. I don't really see those people enabling ALIGN_RODATA anyway. Worse, I expect the loss of RAM efficiently going from 4k-16k pages in a RAM constrained system to be a pretty big hit too. I don't have any hard data one way or the other, and I don't have a strong opinion. Although, I suspect at the moment the potential users of 16k pages may tend toward the small side.
On 12 February 2016 at 17:43, Jeremy Linton <jeremy.linton@arm.com> wrote: > On 02/12/2016 10:28 AM, Ard Biesheuvel wrote: >> >> On 12 February 2016 at 17:21, Jeremy Linton <jeremy.linton@arm.com> wrote: >>> >>> On 02/12/2016 10:11 AM, Ard Biesheuvel wrote: >>>> >>>> >>>> On 12 February 2016 at 17:06, Jeremy Linton <jeremy.linton@arm.com> >>>> wrote: >>> >>> >>> (trimming) >>>>> >>>>> >>>>> #if defined(CONFIG_DEBUG_ALIGN_RODATA) >>>>> -#define ALIGN_DEBUG_RO . = ALIGN(1<<SECTION_SHIFT); >>>>> -#define ALIGN_DEBUG_RO_MIN(min) ALIGN_DEBUG_RO >>>>> +#if defined(CONFIG_ARM64_64K_PAGES) >>>>> +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(CONT_SIZE); >>>>> +#else >>>>> +#define ALIGN_DEBUG_RO_MIN(min) . = >>>>> ALIGN(SECTION_SIZE); >>>> >>>> >>>> >>>> Doesn't this align to 32 MB on 16k pages kernels? >>> >>> >>> >>> Yes, I considered whether it was more appropriate to use CONT_SIZE for >>> 16k >>> as well. >>> >>> Opinions? >>> >> >> Looking at vmlinux.lds.S, I see that that would put _stext and >> __init_begin at 32 MB aligned boundaries. making the size of the >> kernel at least 64 MB. If I take your .rodata patch into account, >> which adds a third instance of ALIGN_DEBUG_RO_MIN, the Image footprint >> will rise to ~100 MB. Or am I missing something? >> > > No, I think your correct. But, its an option, and it sort of depends on use > case. In a system with 100+GB of RAM it might be useful. Not so much on a > phone or small embedded system. I don't really see those people enabling > ALIGN_RODATA anyway. Worse, I expect the loss of RAM efficiently going from > 4k-16k pages in a RAM constrained system to be a pretty big hit too. > > I don't have any hard data one way or the other, and I don't have a strong > opinion. Although, I suspect at the moment the potential users of 16k pages > may tend toward the small side. > Well, the thing is, if you only use 2 MB of each 32 MB on average, you gain nothing by going from contiguous pages to proper sections, since the TLB footprint is the same. So my opinion would be to use CONT_SIZE for 16k pages as well.
On 12 February 2016 at 17:46, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 12 February 2016 at 17:43, Jeremy Linton <jeremy.linton@arm.com> wrote: >> On 02/12/2016 10:28 AM, Ard Biesheuvel wrote: >>> >>> On 12 February 2016 at 17:21, Jeremy Linton <jeremy.linton@arm.com> wrote: >>>> >>>> On 02/12/2016 10:11 AM, Ard Biesheuvel wrote: >>>>> >>>>> >>>>> On 12 February 2016 at 17:06, Jeremy Linton <jeremy.linton@arm.com> >>>>> wrote: >>>> >>>> >>>> (trimming) >>>>>> >>>>>> >>>>>> #if defined(CONFIG_DEBUG_ALIGN_RODATA) >>>>>> -#define ALIGN_DEBUG_RO . = ALIGN(1<<SECTION_SHIFT); >>>>>> -#define ALIGN_DEBUG_RO_MIN(min) ALIGN_DEBUG_RO >>>>>> +#if defined(CONFIG_ARM64_64K_PAGES) >>>>>> +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(CONT_SIZE); >>>>>> +#else >>>>>> +#define ALIGN_DEBUG_RO_MIN(min) . = >>>>>> ALIGN(SECTION_SIZE); >>>>> >>>>> >>>>> >>>>> Doesn't this align to 32 MB on 16k pages kernels? >>>> >>>> >>>> >>>> Yes, I considered whether it was more appropriate to use CONT_SIZE for >>>> 16k >>>> as well. >>>> >>>> Opinions? >>>> >>> >>> Looking at vmlinux.lds.S, I see that that would put _stext and >>> __init_begin at 32 MB aligned boundaries. making the size of the >>> kernel at least 64 MB. If I take your .rodata patch into account, >>> which adds a third instance of ALIGN_DEBUG_RO_MIN, the Image footprint >>> will rise to ~100 MB. Or am I missing something? >>> >> >> No, I think your correct. But, its an option, and it sort of depends on use >> case. In a system with 100+GB of RAM it might be useful. Not so much on a >> phone or small embedded system. I don't really see those people enabling >> ALIGN_RODATA anyway. Worse, I expect the loss of RAM efficiently going from >> 4k-16k pages in a RAM constrained system to be a pretty big hit too. >> >> I don't have any hard data one way or the other, and I don't have a strong >> opinion. Although, I suspect at the moment the potential users of 16k pages >> may tend toward the small side. >> > > Well, the thing is, if you only use 2 MB of each 32 MB on average, you > gain nothing by going from contiguous pages to proper sections, since > the TLB footprint is the same. So my opinion would be to use CONT_SIZE > for 16k pages as well. Actually, none of this matters, since the physical alignment is not guaranteed to be 32 MB. So anything beyond 2 MB is not really meaningful imo
diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug index e13c4bf..65705ee 100644 --- a/arch/arm64/Kconfig.debug +++ b/arch/arm64/Kconfig.debug @@ -59,15 +59,15 @@ config DEBUG_RODATA If in doubt, say Y config DEBUG_ALIGN_RODATA - depends on DEBUG_RODATA && ARM64_4K_PAGES + depends on DEBUG_RODATA bool "Align linker sections up to SECTION_SIZE" help If this option is enabled, sections that may potentially be marked as - read only or non-executable will be aligned up to the section size of - the kernel. This prevents sections from being split into pages and - avoids a potential TLB penalty. The downside is an increase in - alignment and potentially wasted space. Turn on this option if - performance is more important than memory pressure. + read only or non-executable will be aligned up to the section size + or contiguous hint size of the kernel. This prevents sections from + being split into pages and avoids a potential TLB penalty. The downside + is an increase in alignment and potentially wasted space. Turn on + this option if performance is more important than memory pressure. If in doubt, say N diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S index b78a3c7..ab4e436 100644 --- a/arch/arm64/kernel/vmlinux.lds.S +++ b/arch/arm64/kernel/vmlinux.lds.S @@ -63,13 +63,14 @@ PECOFF_FILE_ALIGNMENT = 0x200; #endif #if defined(CONFIG_DEBUG_ALIGN_RODATA) -#define ALIGN_DEBUG_RO . = ALIGN(1<<SECTION_SHIFT); -#define ALIGN_DEBUG_RO_MIN(min) ALIGN_DEBUG_RO +#if defined(CONFIG_ARM64_64K_PAGES) +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(CONT_SIZE); +#else +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(SECTION_SIZE); +#endif #elif defined(CONFIG_DEBUG_RODATA) -#define ALIGN_DEBUG_RO . = ALIGN(1<<PAGE_SHIFT); -#define ALIGN_DEBUG_RO_MIN(min) ALIGN_DEBUG_RO +#define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(PAGE_SIZE); #else -#define ALIGN_DEBUG_RO #define ALIGN_DEBUG_RO_MIN(min) . = ALIGN(min); #endif
This change allows ALIGN_RODATA for 16k and 64k kernels. In the case of 64k kernels it actually aligns to the CONT_SIZE rather than the SECTION_SIZE (which is 512M). This makes it generally more useful, especially for CONT enabled kernels. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> --- arch/arm64/Kconfig.debug | 12 ++++++------ arch/arm64/kernel/vmlinux.lds.S | 11 ++++++----- 2 files changed, 12 insertions(+), 11 deletions(-)