All of lore.kernel.org
 help / color / mirror / Atom feed
From: k.khlebnikov@samsung.com (Konstantin Khlebnikov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RFC] ARM: option for loading modules into vmalloc area
Date: Tue, 18 Nov 2014 21:13:56 +0300	[thread overview]
Message-ID: <546B8C64.3010904@samsung.com> (raw)
In-Reply-To: <20141118173413.GB4042@n2100.arm.linux.org.uk>


On 2014-11-18 20:34, Russell King - ARM Linux wrote:
> On Tue, Nov 18, 2014 at 08:21:46PM +0400, Konstantin Khlebnikov wrote:
>> Usually modules are loaded into small area prior to the kernel
>> text because they are linked with the kernel using short calls.
>> Compile-time instrumentation like GCOV or KASAN bloats code a lot,
>> and as a result huge modules no longer fit into reserved area.
>>
>> This patch adds option CONFIG_MODULES_USE_VMALLOC which lifts
>> limitation on amount of loaded modules. It links modules using
>> long-calls (option -mlong-calls) and loads them into vmalloc area.
>>
>> In few places exported symbols are called from inline assembly.
>> This patch adds macro for such call sites: __asmbl and __asmbl_clobber.
>> Call turns into single 'bl' or sequence 'movw; movt; blx' depending on
>> context and state of config option.
>>
>> Unfortunately this option isn't compatible with CONFIG_FUNCTION_TRACER.
>> Compiler emits short calls to profiling function despite of -mlong-calls.
>> This is a bug in GCC, but ftrace anyway needs an update to handle this.
> It also isn't compatible with the older architectures which don't have
> "blx".

Ok, I'll add "depends on CPU_V6 || CPU_V7" I don't think that it is 
necessary for older cpus.

>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>> index 9d2cdda..9e0c4f4 100644
>> --- a/arch/arm/mm/mmu.c
>> +++ b/arch/arm/mm/mmu.c
>> @@ -1161,16 +1161,15 @@ void __init sanity_check_meminfo(void)
>>   
>>   static inline void prepare_page_table(void)
>>   {
>> -	unsigned long addr;
>> +	unsigned long addr = 0;
>>   	phys_addr_t end;
>>   
>>   	/*
>>   	 * Clear out all the mappings below the kernel image.
>>   	 */
>> -	for (addr = 0; addr < MODULES_VADDR; addr += PMD_SIZE)
>> -		pmd_clear(pmd_off_k(addr));
>> -
>>   #ifdef CONFIG_XIP_KERNEL
>> +	for ( ; addr < MODULES_VADDR; addr += PMD_SIZE)
>> +		pmd_clear(pmd_off_k(addr));
>>   	/* The XIP kernel is mapped in the module area -- skip over it */
>>   	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
>>   #endif
> This seems to be an unrelated change.  In any case, it looks wrong to
> me.  Please describe what you are trying to achieve here.
>

My patch changes  MODULES_VADDR, if option enabled it becomes alias for 
VMALLOC_START

It's used as border between two loops, before patch this code looks like:

     /*
      * Clear out all the mappings below the kernel image.
      */
     for (addr = 0; addr < MODULES_VADDR; addr += PMD_SIZE)
         pmd_clear(pmd_off_k(addr));

#ifdef CONFIG_XIP_KERNEL
     /* The XIP kernel is mapped in the module area -- skip over it */
     addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
#endif
     for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
         pmd_clear(pmd_off_k(addr));

It's written with assumption that MODULES_VADDR < PAGE_OFFSET,
but this no longer true if modules are placed inside vmalloc area.


After patch:

     unsigned long addr = 0;

     /*
      * Clear out all the mappings below the kernel image.
      */
#ifdef CONFIG_XIP_KERNEL
     for ( ; addr < MODULES_VADDR; addr += PMD_SIZE)
         pmd_clear(pmd_off_k(addr));
     /* The XIP kernel is mapped in the module area -- skip over it */
     addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
#endif
     for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
         pmd_clear(pmd_off_k(addr));


Without XIP there is no reason for splitting it into two loops.

WARNING: multiple messages have this Message-ID (diff)
From: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Andrey Ryabinin <a.ryabinin@samsung.com>
Subject: Re: [PATCH RFC] ARM: option for loading modules into vmalloc area
Date: Tue, 18 Nov 2014 21:13:56 +0300	[thread overview]
Message-ID: <546B8C64.3010904@samsung.com> (raw)
In-Reply-To: <20141118173413.GB4042@n2100.arm.linux.org.uk>


On 2014-11-18 20:34, Russell King - ARM Linux wrote:
> On Tue, Nov 18, 2014 at 08:21:46PM +0400, Konstantin Khlebnikov wrote:
>> Usually modules are loaded into small area prior to the kernel
>> text because they are linked with the kernel using short calls.
>> Compile-time instrumentation like GCOV or KASAN bloats code a lot,
>> and as a result huge modules no longer fit into reserved area.
>>
>> This patch adds option CONFIG_MODULES_USE_VMALLOC which lifts
>> limitation on amount of loaded modules. It links modules using
>> long-calls (option -mlong-calls) and loads them into vmalloc area.
>>
>> In few places exported symbols are called from inline assembly.
>> This patch adds macro for such call sites: __asmbl and __asmbl_clobber.
>> Call turns into single 'bl' or sequence 'movw; movt; blx' depending on
>> context and state of config option.
>>
>> Unfortunately this option isn't compatible with CONFIG_FUNCTION_TRACER.
>> Compiler emits short calls to profiling function despite of -mlong-calls.
>> This is a bug in GCC, but ftrace anyway needs an update to handle this.
> It also isn't compatible with the older architectures which don't have
> "blx".

Ok, I'll add "depends on CPU_V6 || CPU_V7" I don't think that it is 
necessary for older cpus.

>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>> index 9d2cdda..9e0c4f4 100644
>> --- a/arch/arm/mm/mmu.c
>> +++ b/arch/arm/mm/mmu.c
>> @@ -1161,16 +1161,15 @@ void __init sanity_check_meminfo(void)
>>   
>>   static inline void prepare_page_table(void)
>>   {
>> -	unsigned long addr;
>> +	unsigned long addr = 0;
>>   	phys_addr_t end;
>>   
>>   	/*
>>   	 * Clear out all the mappings below the kernel image.
>>   	 */
>> -	for (addr = 0; addr < MODULES_VADDR; addr += PMD_SIZE)
>> -		pmd_clear(pmd_off_k(addr));
>> -
>>   #ifdef CONFIG_XIP_KERNEL
>> +	for ( ; addr < MODULES_VADDR; addr += PMD_SIZE)
>> +		pmd_clear(pmd_off_k(addr));
>>   	/* The XIP kernel is mapped in the module area -- skip over it */
>>   	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
>>   #endif
> This seems to be an unrelated change.  In any case, it looks wrong to
> me.  Please describe what you are trying to achieve here.
>

My patch changes  MODULES_VADDR, if option enabled it becomes alias for 
VMALLOC_START

It's used as border between two loops, before patch this code looks like:

     /*
      * Clear out all the mappings below the kernel image.
      */
     for (addr = 0; addr < MODULES_VADDR; addr += PMD_SIZE)
         pmd_clear(pmd_off_k(addr));

#ifdef CONFIG_XIP_KERNEL
     /* The XIP kernel is mapped in the module area -- skip over it */
     addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
#endif
     for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
         pmd_clear(pmd_off_k(addr));

It's written with assumption that MODULES_VADDR < PAGE_OFFSET,
but this no longer true if modules are placed inside vmalloc area.


After patch:

     unsigned long addr = 0;

     /*
      * Clear out all the mappings below the kernel image.
      */
#ifdef CONFIG_XIP_KERNEL
     for ( ; addr < MODULES_VADDR; addr += PMD_SIZE)
         pmd_clear(pmd_off_k(addr));
     /* The XIP kernel is mapped in the module area -- skip over it */
     addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
#endif
     for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
         pmd_clear(pmd_off_k(addr));


Without XIP there is no reason for splitting it into two loops.

  reply	other threads:[~2014-11-18 18:13 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-18 16:21 [PATCH RFC] ARM: option for loading modules into vmalloc area Konstantin Khlebnikov
2014-11-18 16:21 ` Konstantin Khlebnikov
2014-11-18 17:34 ` Russell King - ARM Linux
2014-11-18 17:34   ` Russell King - ARM Linux
2014-11-18 18:13   ` Konstantin Khlebnikov [this message]
2014-11-18 18:13     ` Konstantin Khlebnikov
2014-11-19 13:40     ` Arnd Bergmann
2014-11-19 13:40       ` Arnd Bergmann
2014-11-19 14:54       ` Ard Biesheuvel
2014-11-19 14:54         ` Ard Biesheuvel
2014-11-19 15:52         ` Konstantin Khlebnikov
2014-11-19 15:52           ` Konstantin Khlebnikov
2014-11-19 16:02           ` Ard Biesheuvel
2014-11-19 16:02             ` Ard Biesheuvel
2014-11-19 16:07             ` Russell King - ARM Linux
2014-11-19 16:07               ` Russell King - ARM Linux
2014-11-19 16:25               ` Ard Biesheuvel
2014-11-19 16:25                 ` Ard Biesheuvel
2014-11-19 16:32                 ` Konstantin Khlebnikov
2014-11-19 16:32                   ` Konstantin Khlebnikov
2014-11-19 16:38                   ` Ard Biesheuvel
2014-11-19 16:38                     ` Ard Biesheuvel
2014-11-19 16:55                     ` Russell King - ARM Linux
2014-11-19 16:55                       ` Russell King - ARM Linux
2014-11-19 16:59                       ` Nicolas Pitre
2014-11-19 16:59                         ` Nicolas Pitre
2014-11-19 16:41                 ` Russell King - ARM Linux
2014-11-19 16:41                   ` Russell King - ARM Linux
2014-11-19 16:37               ` Nicolas Pitre
2014-11-19 16:37                 ` Nicolas Pitre
2014-11-19 16:41                 ` Ard Biesheuvel
2014-11-19 16:41                   ` Ard Biesheuvel
2014-11-19 16:49                 ` Russell King - ARM Linux
2014-11-19 16:49                   ` Russell King - ARM Linux
2014-11-19 16:57                   ` Nicolas Pitre
2014-11-19 16:57                     ` Nicolas Pitre
2014-11-19 16:59                     ` Russell King - ARM Linux
2014-11-19 16:59                       ` Russell King - ARM Linux
2014-11-19 17:12                       ` Nicolas Pitre
2014-11-19 17:12                         ` Nicolas Pitre
2014-11-19 17:59                         ` Ard Biesheuvel
2014-11-19 17:59                           ` Ard Biesheuvel
2014-11-19 18:22                           ` Nicolas Pitre
2014-11-19 18:22                             ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=546B8C64.3010904@samsung.com \
    --to=k.khlebnikov@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.