bpf_prog_pack followup

Introduce bpf_arch_text_invalidate and use it to fill unused part of the bpf_prog_pack with illegal instructions when a BPF program is freed.

Signed-off-by Song Liu <[email protected]>

---
 arch/x86/net/bpf_jit_comp.c | 5 +++++
 include/linux/bpf.h         | 1 +
 kernel/bpf/core.c           | 8 ++++++++
 3 files changed, 14 insertions(+)

arch/x86/net/bpf_jit_comp.c +5 -0

index a2b6d197c226..f298b18a9a3d 100644
@@ -228,6 +228,11 @@ static void jit_fill_hole(void *area, unsigned int size)
 	memset(area, 0xcc, size);
 }
 
+int bpf_arch_text_invalidate(void *dst, size_t len)
+{
+	return IS_ERR_OR_NULL(text_poke_set(dst, 0xcc, len));
+}
+
 struct jit_context {
 	int cleanup_addr; /* Epilogue code offset */
 

include/linux/bpf.h +1 -0

index cc4d5e394031..a9b1875212f6 100644
@@ -2365,6 +2365,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
 		       void *addr1, void *addr2);
 
 void *bpf_arch_text_copy(void *dst, void *src, size_t len);
+int bpf_arch_text_invalidate(void *dst, size_t len);
 
 struct btf_id_set;
 bool btf_id_set_contains(const struct btf_id_set *set, u32 id);

kernel/bpf/core.c +0 -0

index 2d0c9d4696ad..cacd8684c3c4 100644

kernel/bpf/core.c +6 -0

+
 	bitmap_clear(pack->bitmap, pos, nbits);
 	if (bitmap_find_next_zero_area(pack->bitmap, bpf_prog_chunk_count(), 0,
 				       bpf_prog_chunk_count(), 0) == 0) {

@@ -2740,6 +2743,11 @@ void * __weak bpf_arch_text_copy(void *dst, void *src, size_t len)
 	return ERR_PTR(-ENOTSUPP);
 }
 
+int __weak bpf_arch_text_invalidate(void *dst, size_t len)
+{
+	return -ENOTSUPP;
+}
+
 DEFINE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
 EXPORT_SYMBOL(bpf_stats_enabled_key);
 

Signature · 1 line

2.30.2

Use module_alloc_huge for bpf_prog_pack so that BPF programs sit on PMD_SIZE pages. This benefits system performance by reducing iTLB miss rate. Benchmark of a real web service workload shows this change gives another ~0.2% performance boost on top of PAGE_SIZE bpf_prog_pack (which improve system throughput by ~0.5%).

Also, remove set_vm_flush_reset_perms() from alloc_new_pack() and use set_memory_[nx|rw] in bpf_prog_pack_free(). This is because VM_FLUSH_RESET_PERMS does not work with huge pages yet. [1]

[1] https://lore.kernel.org/bpf/[email protected]/

Suggested-by Rick Edgecombe <[email protected]>

Signed-off-by Song Liu <[email protected]>

---
 kernel/bpf/core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

kernel/bpf/core.c +3 -5

index cacd8684c3c4..b64d91fcb0ba 100644
@@ -857,7 +857,7 @@ static size_t select_bpf_prog_pack_size(void)
 	void *ptr;
 
 	size = BPF_HPAGE_SIZE * num_online_nodes();
-	ptr = module_alloc(size);
+	ptr = module_alloc_huge(size);
 
 	/* Test whether we can get huge pages. If not just use PAGE_SIZE
 	 * packs.

@@ -881,7 +881,7 @@ static struct bpf_prog_pack *alloc_new_pack(bpf_jit_fill_hole_t bpf_fill_ill_ins
 		       GFP_KERNEL);
 	if (!pack)
 		return NULL;
-	pack->ptr = module_alloc(bpf_prog_pack_size);
+	pack->ptr = module_alloc_huge(bpf_prog_pack_size);
 	if (!pack->ptr) {
 		kfree(pack);
 		return NULL;

@@ -890,7 +890,6 @@ static struct bpf_prog_pack *alloc_new_pack(bpf_jit_fill_hole_t bpf_fill_ill_ins
 	bitmap_zero(pack->bitmap, bpf_prog_pack_size / BPF_PROG_CHUNK_SIZE);
 	list_add_tail(&pack->list, &pack_list);
 
-	set_vm_flush_reset_perms(pack->ptr);
 	set_memory_ro((unsigned long)pack->ptr, bpf_prog_pack_size / PAGE_SIZE);
 	set_memory_x((unsigned long)pack->ptr, bpf_prog_pack_size / PAGE_SIZE);
 	return pack;

@@ -909,10 +908,9 @@ static void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insn
 
 	if (size > bpf_prog_pack_size) {
 		size = round_up(size, PAGE_SIZE);
-		ptr = module_alloc(size);
+		ptr = module_alloc_huge(size);
 		if (ptr) {
 			bpf_fill_ill_insns(ptr, size);
-			set_vm_flush_reset_perms(ptr);
 			set_memory_ro((unsigned long)ptr, size / PAGE_SIZE);
 			set_memory_x((unsigned long)ptr, size / PAGE_SIZE);
 		}

kernel/bpf/core.c +0 -0

 		module_memfree(pack->ptr);
 		kfree(pack);
 	}

Signature · 1 line

2.30.2

Review static void bpf_prog_pack_free(struct · line 954

Quoted 9 lines

(quoted diff) +2 -0

@@ -949,6 +947,8 @@ static void bpf_prog_pack_free(struct
 bpf_binary_header *hdr)
 
       mutex_lock(&pack_mutex);
       if (hdr->size > bpf_prog_pack_size) {
+             set_memory_nx((unsigned long)hdr, hdr->size /
 PAGE_SIZE);
+             set_memory_rw((unsigned long)hdr, hdr->size /
 PAGE_SIZE);

set_memory_{nx,rw} can fail. Please take care of error handling.

Use module_alloc_huge for bpf_prog_pack so that BPF programs sit on PMD_SIZE pages. This benefits system performance by reducing iTLB miss
rate. Benchmark of a real web service workload shows this change gives
another ~0.2% performance boost on top of PAGE_SIZE bpf_prog_pack (which improve system throughput by ~0.5%).

Also, remove set_vm_flush_reset_perms() from alloc_new_pack() and use set_memory_[nx|rw] in bpf_prog_pack_free(). This is because VM_FLUSH_RESET_PERMS does not work with huge pages yet. [1]

[1]
https://lore.kernel.org/bpf/[email protected]/

Suggested-by Rick Edgecombe <[email protected]>

Signed-off-by Song Liu <[email protected]>

The primary goal of bpf_prog_pack is to reduce iTLB miss rate and reduce direct memory mapping fragmentation. This leads to non-trivial performance improvements.

For our web service production benchmark, bpf_prog_pack on 4kB pages gives 0.5% to 0.7% more throughput than not using bpf_prog_pack. bpf_prog_pack on 2MB pages 0.6% to 0.9% more throughput than not using bpf_prog_pack. Note that 0.5% is a huge improvement for our fleet. I believe this is also significant for other companies with many thousand servers.

Hi Song,

On Fri, May 20, 2022 at 04:57:50PM -0700, Song Liu wrote:

... ...

Quoted 11 lines

The primary goal of bpf_prog_pack is to reduce iTLB miss rate and reduce direct memory mapping fragmentation. This leads to non-trivial performance improvements.

For our web service production benchmark, bpf_prog_pack on 4kB pages gives 0.5% to 0.7% more throughput than not using bpf_prog_pack. bpf_prog_pack on 2MB pages 0.6% to 0.9% more throughput than not using bpf_prog_pack. Note that 0.5% is a huge improvement for our fleet. I believe this is also significant for other companies with many thousand servers.

I'm evaluationg performance impact due to direct memory mapping fragmentation and seeing the above, I wonder: is the performance improve mostly due to prog pack and hugepage instead of less direct mapping fragmentation?

I can understand that when progs are packed together, iTLB miss rate will be reduced and thus, performance can be improved. But I don't see immediately how direct mapping fragmentation can impact performance since the bpf code are running from the module alias addresses, not the direct mapping addresses IIUC?

Hi Aaron,

Quoted 24 lines · Aaron Lu

Hi Song,

On Fri, May 20, 2022 at 04:57:50PM -0700, Song Liu wrote:

... ...

Quoted 11 lines

The primary goal of bpf_prog_pack is to reduce iTLB miss rate and reduce direct memory mapping fragmentation. This leads to non-trivial performance improvements.

For our web service production benchmark, bpf_prog_pack on 4kB pages gives 0.5% to 0.7% more throughput than not using bpf_prog_pack. bpf_prog_pack on 2MB pages 0.6% to 0.9% more throughput than not using bpf_prog_pack. Note that 0.5% is a huge improvement for our fleet. I believe this is also significant for other companies with many thousand servers.

I'm evaluationg performance impact due to direct memory mapping fragmentation and seeing the above, I wonder: is the performance improve mostly due to prog pack and hugepage instead of less direct mapping fragmentation?

Quoted 5 lines

I can understand that when progs are packed together, iTLB miss rate will be reduced and thus, performance can be improved. But I don't see immediately how direct mapping fragmentation can impact performance since the bpf code are running from the module alias addresses, not the direct mapping addresses IIUC?

You are right that BPF code runs from module alias addresses. However, to protect text from overwrites, we use set_memory_x() and set_memory_ro() for the BPF code. These two functions will set permissions for all aliases of the memory, including the direct map, and thus cause fragmentation of the direct map. Does this make sense?

Quoted 33 lines · Song Liu

Hi Aaron,

Quoted 24 lines · Aaron Lu

Hi Song,

On Fri, May 20, 2022 at 04:57:50PM -0700, Song Liu wrote:

... ...

Quoted 11 lines

The primary goal of bpf_prog_pack is to reduce iTLB miss rate and reduce direct memory mapping fragmentation. This leads to non-trivial performance improvements.

For our web service production benchmark, bpf_prog_pack on 4kB pages gives 0.5% to 0.7% more throughput than not using bpf_prog_pack. bpf_prog_pack on 2MB pages 0.6% to 0.9% more throughput than not using bpf_prog_pack. Note that 0.5% is a huge improvement for our fleet. I believe this is also significant for other companies with many thousand servers.

I'm evaluationg performance impact due to direct memory mapping fragmentation and seeing the above, I wonder: is the performance improve mostly due to prog pack and hugepage instead of less direct mapping fragmentation?

Quoted 5 lines

I can understand that when progs are packed together, iTLB miss rate will be reduced and thus, performance can be improved. But I don't see immediately how direct mapping fragmentation can impact performance since the bpf code are running from the module alias addresses, not the direct mapping addresses IIUC?

Quoted 5 lines

You are right that BPF code runs from module alias addresses. However, to protect text from overwrites, we use set_memory_x() and set_memory_ro() for the BPF code. These two functions will set permissions for all aliases of the memory, including the direct map, and thus cause fragmentation of the direct map. Does this make sense?

Guess I didn't make it clear.

I understand that set_memory_XXX() will cause direct mapping split and thus, fragmented. What is not clear to me is, how much impact does direct mapping fragmentation have on performance, in your case and in general?

In your case, I guess the performance gain is due to code gets packed together and iTLB gets reduced. When code are a lot, packing them together as a hugepage is a further gain. In the meantime, direct mapping split (or not) seems to be a side effect of this packing, but it doesn't have a direct impact on performance.

One thing I can imagine is, when an area of direct mapping gets splited due to permission reason, when that reason is gone(like module unload or bpf code unload), those areas will remain fragmented and that can cause later operations that touch these same areas using more dTLBs and that can be bad for performance, but it's hard to say how much impact this can cause though.

Quoted 54 lines · Aaron Lu

Quoted 33 lines · Song Liu

Hi Aaron,

Quoted 24 lines · Aaron Lu

Hi Song,

On Fri, May 20, 2022 at 04:57:50PM -0700, Song Liu wrote:

... ...

Quoted 11 lines

The primary goal of bpf_prog_pack is to reduce iTLB miss rate and reduce direct memory mapping fragmentation. This leads to non-trivial performance improvements.

For our web service production benchmark, bpf_prog_pack on 4kB pages gives 0.5% to 0.7% more throughput than not using bpf_prog_pack. bpf_prog_pack on 2MB pages 0.6% to 0.9% more throughput than not using bpf_prog_pack. Note that 0.5% is a huge improvement for our fleet. I believe this is also significant for other companies with many thousand servers.

I'm evaluationg performance impact due to direct memory mapping fragmentation and seeing the above, I wonder: is the performance improve mostly due to prog pack and hugepage instead of less direct mapping fragmentation?

Quoted 5 lines

I can understand that when progs are packed together, iTLB miss rate will be reduced and thus, performance can be improved. But I don't see immediately how direct mapping fragmentation can impact performance since the bpf code are running from the module alias addresses, not the direct mapping addresses IIUC?

Quoted 5 lines

You are right that BPF code runs from module alias addresses. However, to protect text from overwrites, we use set_memory_x() and set_memory_ro() for the BPF code. These two functions will set permissions for all aliases of the memory, including the direct map, and thus cause fragmentation of the direct map. Does this make sense?

Guess I didn't make it clear.

I understand that set_memory_XXX() will cause direct mapping split and thus, fragmented. What is not clear to me is, how much impact does direct mapping fragmentation have on performance, in your case and in general?

In your case, I guess the performance gain is due to code gets packed together and iTLB gets reduced. When code are a lot, packing them together as a hugepage is a further gain. In the meantime, direct mapping split (or not) seems to be a side effect of this packing, but it doesn't have a direct impact on performance.

Quoted 6 lines

One thing I can imagine is, when an area of direct mapping gets splited due to permission reason, when that reason is gone(like module unload or bpf code unload), those areas will remain fragmented and that can cause later operations that touch these same areas using more dTLBs and that can be bad for performance, but it's hard to say how much impact this can cause though.

Yes, we have data showing the direct mapping remaining fragmented can cause non-trivial performance degradation. For our web workload, the difference is in the order of 1%.

Hi Song,

On Fri, May 20, 2022 at 04:57:50PM -0700, Song Liu wrote:

... ...

Quoted 11 lines

The primary goal of bpf_prog_pack is to reduce iTLB miss rate and reduce direct memory mapping fragmentation. This leads to non-trivial performance improvements.

For our web service production benchmark, bpf_prog_pack on 4kB pages gives 0.5% to 0.7% more throughput than not using bpf_prog_pack. bpf_prog_pack on 2MB pages 0.6% to 0.9% more throughput than not using bpf_prog_pack. Note that 0.5% is a huge improvement for our fleet. I believe this is also significant for other companies with many thousand servers.

I'm evaluationg performance impact due to direct memory mapping fragmentation

and seeing the above, I wonder: is the performance improve mostly due to prog pack and hugepage instead of less direct mapping fragmentation?

I can understand that when progs are packed together, iTLB miss rate will be reduced and thus, performance can be improved. But I don't see immediately how direct mapping fragmentation can impact performance since the bpf code are running from the module alias addresses, not the direct mapping addresses IIUC?

I appreciate it if you can shed some light on performance impact direct mapping fragmentation can cause, thanks.

Quoted 18 lines · Aaron Lu

Hi Song,

On Fri, May 20, 2022 at 04:57:50PM -0700, Song Liu wrote:

... ...

Quoted 11 lines

The primary goal of bpf_prog_pack is to reduce iTLB miss rate and reduce direct memory mapping fragmentation. This leads to non-trivial performance improvements.

For our web service production benchmark, bpf_prog_pack on 4kB pages gives 0.5% to 0.7% more throughput than not using bpf_prog_pack. bpf_prog_pack on 2MB pages 0.6% to 0.9% more throughput than not using bpf_prog_pack. Note that 0.5% is a huge improvement for our fleet. I believe this is also significant for other companies with many thousand servers.

Quoted 2 lines

I'm evaluationg performance impact due to direct memory mapping fragmentation

BTW how exactly are you doing this?