All of lore.kernel.org
 help / color / mirror / Atom feed
From: Song Liu <songliubraving@fb.com>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: Song Liu <song@kernel.org>, lkml <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"x86@vger.kernel.org" <x86@vger.kernel.org>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"rick.p.edgecombe@intel.com" <rick.p.edgecombe@intel.com>,
	Kernel Team <Kernel-team@fb.com>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>
Subject: Re: [PATCH v5 bpf-next 1/5] module: introduce module_alloc_huge
Date: Wed, 6 Jul 2022 04:39:13 +0000	[thread overview]
Message-ID: <16959523-ABD1-4D2B-B249-118DDADD7976@fb.com> (raw)
In-Reply-To: <Yr+BV+HLZikpCU42@bombadil.infradead.org>



> On Jul 1, 2022, at 4:20 PM, Luis Chamberlain <mcgrof@kernel.org> wrote:
> 
> On Fri, Jun 24, 2022 at 02:57:08PM -0700, Song Liu wrote:
>> Introduce module_alloc_huge, which allocates huge page backed memory in
>> module memory space. The primary user of this memory is bpf_prog_pack
>> (multiple BPF programs sharing a huge page).
>> 
>> Signed-off-by: Song Liu <song@kernel.org>
> 
> I see mm not Cc'd. I'd like review from them.

I will CC mm in the next version (or resend). Thanks for the reminder. 

> 
>> ---
>> arch/x86/kernel/module.c | 21 +++++++++++++++++++++
>> include/linux/moduleloader.h | 5 +++++
>> kernel/module/main.c | 8 ++++++++
>> 3 files changed, 34 insertions(+)
>> 
>> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
>> index b98ffcf4d250..63f6a16c70dc 100644
>> --- a/arch/x86/kernel/module.c
>> +++ b/arch/x86/kernel/module.c
>> @@ -86,6 +86,27 @@ void *module_alloc(unsigned long size)
>> 	return p;
>> }
>> 
>> +void *module_alloc_huge(unsigned long size)
>> +{
>> +	gfp_t gfp_mask = GFP_KERNEL;
>> +	void *p;
>> +
>> +	if (PAGE_ALIGN(size) > MODULES_LEN)
>> +		return NULL;
>> +
>> +	p = __vmalloc_node_range(size, MODULE_ALIGN,
>> +				 MODULES_VADDR + get_module_load_offset(),
>> +				 MODULES_END, gfp_mask, PAGE_KERNEL,
>> +				 VM_DEFER_KMEMLEAK | VM_ALLOW_HUGE_VMAP,
>> +				 NUMA_NO_NODE, __builtin_return_address(0));
>> +	if (p && (kasan_alloc_module_shadow(p, size, gfp_mask) < 0)) {
>> +		vfree(p);
>> +		return NULL;
>> +	}
>> +
>> +	return p;
>> +}
> 
> 1) When things like kernel/bpf/core.c start using a module alloc it
> is time to consider genearlizing this.

I am not quite sure what the generalization would look like. IMHO, the
ideal case would have:
  a) A kernel_text_rw_allocator, similar to current module_alloc.
  b) A kernel_text_ro_allocator, similar to current bpf_prog_pack_alloc.
     This is built on top of kernel_text_rw_allocator. Different 
     allocations could share a page, thus it requires text_poke like 
     support from the arch. 
  c) If the arch supports text_poke, kprobes, ftrace trampolines, and
     bpf trampolines should use kernel_text_ro_allocator.
  d) Major archs should support CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC,
     and they should use kernel_text_ro_allocator for module text. 

Does this sound reasonable to you?

I tried to enable CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC for x86_64, 
but that doesn't really work. Do we have plan to make this combination
work?

> 
> 2) How we free is important, and each arch does something funky for
> this. This is not addressed here.

How should we address this? IIUC, x86_64 just calls vfree. 

> 
> And yes I welcome generalizing generic module_alloc() too as suggested
> before. The concern on my part is the sloppiness this enables.

One question I have is, does module_alloc (or kernel_text_*_allocator 
above) belong to module code, or mm code (maybe vmalloc)?

I am planning to let BPF trampoline use bpf_prog_pack on x86_64, which 
is another baby step of c) above. 

Thanks,
Song


  reply	other threads:[~2022-07-06  4:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-24 21:57 [PATCH v5 bpf-next 0/5] bpf_prog_pack followup Song Liu
2022-06-24 21:57 ` [PATCH v5 bpf-next 1/5] module: introduce module_alloc_huge Song Liu
2022-07-01 23:20   ` Luis Chamberlain
2022-07-06  4:39     ` Song Liu [this message]
2022-07-07 20:11       ` Luis Chamberlain
2022-06-24 21:57 ` [PATCH v5 bpf-next 2/5] bpf: use module_alloc_huge for bpf_prog_pack Song Liu
2022-06-24 21:57 ` [PATCH v5 bpf-next 3/5] vmalloc: WARN for set_vm_flush_reset_perms() on huge pages Song Liu
2022-06-24 21:57 ` [PATCH v5 bpf-next 4/5] vmalloc: introduce huge_vmalloc_supported Song Liu
2022-06-24 21:57 ` [PATCH v5 bpf-next 5/5] bpf: simplify select_bpf_prog_pack_size Song Liu
2022-06-24 22:00 ` [PATCH v5 bpf-next 0/5] bpf_prog_pack followup Song Liu

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=16959523-ABD1-4D2B-B249-118DDADD7976@fb.com \
    --to=songliubraving@fb.com \
    --cc=Kernel-team@fb.com \
    --cc=daniel@iogearbox.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=song@kernel.org \
    --cc=x86@vger.kernel.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.