lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <dd6dee71-94d7-5393-8fe6-c667938ebfac@iogearbox.net> Date: Fri, 11 Feb 2022 15:35:31 +0100 From: Daniel Borkmann <daniel@...earbox.net> To: Song Liu <songliubraving@...com> Cc: Song Liu <song@...nel.org>, Linux Memory Management List <linux-mm@...ck.org>, bpf <bpf@...r.kernel.org>, Network Development <netdev@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>, Andrii Nakryiko <andrii@...nel.org>, Kernel Team <Kernel-team@...com>, Andrew Morton <akpm@...ux-foundation.org>, Eric Dumazet <eric.dumazet@...il.com>, Michal Hocko <mhocko@...e.com> Subject: Re: [PATCH bpf-next 2/2] bpf: flexible size for bpf_prog_pack On 2/10/22 5:51 PM, Song Liu wrote: >> On Feb 10, 2022, at 12:25 AM, Daniel Borkmann <daniel@...earbox.net> wrote: >> On 2/10/22 7:41 AM, Song Liu wrote: >>> bpf_prog_pack uses huge pages to reduce pressue on instruction TLB. >>> To guarantee allocating huge pages for bpf_prog_pack, it is necessary to >>> allocate memory of size PMD_SIZE * num_online_nodes(). >>> On the other hand, if the system doesn't support huge pages, it is more >>> efficient to allocate PAGE_SIZE bpf_prog_pack. >>> Address different scenarios with more flexible bpf_prog_pack_size(). >>> Signed-off-by: Song Liu <song@...nel.org> >>> --- >>> kernel/bpf/core.c | 47 +++++++++++++++++++++++++++-------------------- >>> 1 file changed, 27 insertions(+), 20 deletions(-) >>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c >>> index 42d96549a804..d961a1f07a13 100644 >>> --- a/kernel/bpf/core.c >>> +++ b/kernel/bpf/core.c >>> @@ -814,46 +814,53 @@ int bpf_jit_add_poke_descriptor(struct bpf_prog *prog, >>> * allocator. The prog_pack allocator uses HPAGE_PMD_SIZE page (2MB on x86) >>> * to host BPF programs. >>> */ >>> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE >>> -#define BPF_PROG_PACK_SIZE HPAGE_PMD_SIZE >>> -#else >>> -#define BPF_PROG_PACK_SIZE PAGE_SIZE >>> -#endif >>> #define BPF_PROG_CHUNK_SHIFT 6 >>> #define BPF_PROG_CHUNK_SIZE (1 << BPF_PROG_CHUNK_SHIFT) >>> #define BPF_PROG_CHUNK_MASK (~(BPF_PROG_CHUNK_SIZE - 1)) >>> -#define BPF_PROG_CHUNK_COUNT (BPF_PROG_PACK_SIZE / BPF_PROG_CHUNK_SIZE) >>> struct bpf_prog_pack { >>> struct list_head list; >>> void *ptr; >>> - unsigned long bitmap[BITS_TO_LONGS(BPF_PROG_CHUNK_COUNT)]; >>> + unsigned long bitmap[]; >>> }; >>> -#define BPF_PROG_MAX_PACK_PROG_SIZE BPF_PROG_PACK_SIZE >>> #define BPF_PROG_SIZE_TO_NBITS(size) (round_up(size, BPF_PROG_CHUNK_SIZE) / BPF_PROG_CHUNK_SIZE) >>> static DEFINE_MUTEX(pack_mutex); >>> static LIST_HEAD(pack_list); >>> +static inline int bpf_prog_pack_size(void) >>> +{ >>> + /* If vmap_allow_huge == true, use pack size of the smallest >>> + * possible vmalloc huge page: PMD_SIZE * num_online_nodes(). >>> + * Otherwise, use pack size of PAGE_SIZE. >>> + */ >>> + return get_vmap_allow_huge() ? PMD_SIZE * num_online_nodes() : PAGE_SIZE; >>> +} >> >> Imho, this is making too many assumptions about implementation details. Can't we >> just add a new module_alloc*() API instead which internally guarantees allocating >> huge pages when enabled/supported (e.g. with a __weak function as fallback)? > > I agree that this is making too many assumptions. But a new module_alloc_huge() > may not work, because we need the caller to know the proper size to ask for. > (Or maybe I misunderstood your suggestion?) > > How about we introduce something like > > /* minimal size to get huge pages from vmalloc. If not possible, > * return 0 (or -1?) > */ > int vmalloc_hpage_min_size(void) > { > return vmap_allow_huge ? PMD_SIZE * num_online_nodes() : 0; > } And that would live inside mm/vmalloc.c and is exported to users ... > /* minimal size to get huge pages from module_alloc */ > int module_alloc_hpage_min_size(void) > { > return vmalloc_hpage_min_size(); > } ... and this one as wrapper in module alloc infra with __weak attr? > static inline int bpf_prog_pack_size(void) > { > return module_alloc_hpage_min_size() ? : PAGE_SIZE; > } Could probably work. It's not nice, but at least in the corresponding places so it's not exposed / hard coded inside bpf and assuming implementation details which could potentially break later on. Thanks, Daniel
Powered by blists - more mailing lists