Re: Error on loading some network Kernel modules

[Vincent Chen <vincent.chen@sifive.com>]

On Fri, Feb 7, 2020 at 10:39 PM Vincent Chen <vincent.chen@sifive.com> wrote:
>
> > > For point1, we don't need a dedicated region as long as we are allocating
> > > modules from VMALLOC area. Let's avoid another virtual memory region if
> > > possible.
> > >
> >
> > Your comments inspire me if we can know the end of the percpu data
> > region, we can dynamically calculate the start address in each module
> > allocation by "<end of the percpu data region> - 2GB" if needed. I am
> > finding a way to derive the <end of the percpu data region>.
> >
>
> In the implementation of the idea, I found that I did have some
> misunderstandings about the mechanism of accessing static percpu
> symbol. Currently, I think this issue can be solved by modifying the
> module_allocate() to the following.
> #define MODULE_START max(PFN_ALIGN(&_end - 2 *
> SZ_1G), PFN_ALIGN(&_text + 8 * SZ_1K - 2 * SZ_1G))
> void *module_alloc(unsigned long size)
> {
> return __vmalloc_node_range(size, 1, MODULE_START,
> VMALLOC_END, GFP_KERNEL,
> PAGE_KERNEL_EXEC, 0,
> NUMA_NO_NODE,
> __builtin_return_address(0));
> }
> After applying a similar modification above, the kernel can
> successfully insert the test module provided by Romain. Any feedback
> is welcome. If someone is curious about the reason, I roughly
> described it at the end of this mail. I am sorry if someone was misled
> by early discussion.
>
>
> As mentioned early, kernel pre-allocates a memory region for each CPU
> to place the instances of the percpu data. According to the
> declaration type, each memory region can be divided into three chunks.
> These three chunks from low memory to high memory are static chunk,
> reserved chunk, and dynamic chunk.
> a. The static chunk is used for the static percpu symbols declared in the kernel
> b. The reserved chunk is used for the static percpu symbols declared
> in the kernel module
> c. The dynamic chunk is used for all dynamic percpu symbols.
>
> The order of the percpu symbols in each memory region is the same and
> is recorded in a symbol list maintained by the kernel. Therefore, the
> address of a specific CPU's percpu symbol is "The address of this
> percpu symbol in the symbol list" + "The offset from symbols list to
> the specific CPU region".
> a. The symbol list is created based on the ".data..percpu" section. In
> other words, the start address of this symbol list is the start
> address of ".data..percpu". When a dynamic percpu symbol is created or
> a kernel module import a static percpu symbol, the kernel will follow
> the above chunk rule to add this percpu symbol to its suitable region.
> b. When each memory area was created, "The offset from symbols list to
> the specific CPU region" has been recorded.
>
> Back to the issue, according to the rules above, the kernel will
> relocate the ".data..percpu" section of the loaded module to the end
> of the ".data..percpu" section of the kernel image. The end of the
> ".data..percpu" section of the kernel image almost equals _text, and
> the current size of the reserved chunk is 8KB. Therefore, the
> appropriate range to place the loaded module is [max(PFN_ALIGN(&_end -
> 2 *SZ_1G), PFN_ALIGN(&_text + 8 * SZ_1K - 2 * SZ_1G)),  VMALLOC_END].

I did not notice that the size of the kernel .text section is
impossible below 2 PAGES. Therefore, the modification of module_alloc
function can be reduced to the following.
void *module_alloc(unsigned long size)
 {
 return __vmalloc_node_range(size, 1, PFN_ALIGN(&_end - 2 *SZ_1G),
 VMALLOC_END, GFP_KERNEL,
 PAGE_KERNEL_EXEC, 0,
 NUMA_NO_NODE,
 __builtin_return_address(0));
 }
It is the same as Alex's suggestion. Is Alex willing to send this
patch to resolve this bug?