diff mbox series

[SRU,Focal] Revert "bpf: Zero-fill re-used per-cpu map element"

Message ID 20201204184757.9968-1-kamal@canonical.com
State New
Headers show
Series [SRU,Focal] Revert "bpf: Zero-fill re-used per-cpu map element" | expand

Commit Message

Kamal Mostafa Dec. 4, 2020, 6:47 p.m. UTC 
BugLink: https://bugs.launchpad.net/bugs/1906866

This reverts commit d946d4ddd6af531398201e7ce8f73bd1d8a98e2a.

Reported upstream:
 https://lore.kernel.org/stable/20201204182846.27110-1-kamal@canonical.com/T/#u

This v5.4.78 commit breaks the tools/testing/selftests/bpf build:

[linux-5.4.y] c602ad2b52dc bpf: Zero-fill re-used per-cpu map element
[focal] d946d4ddd6af bpf: Zero-fill re-used per-cpu map element

Like this:

        prog_tests/map_init.c:5:10: fatal error: test_map_init.skel.h:
No such file or directory
            5 | #include "test_map_init.skel.h"

Because tools/testing/selftests/bpf/Makefile in v5.4 does not have the
"skeleton header generation" stuff (circa v5.6).

Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 kernel/bpf/hashtab.c                          |  30 +--
 .../selftests/bpf/prog_tests/map_init.c       | 214 ------------------
 .../selftests/bpf/progs/test_map_init.c       |  33 ---
 3 files changed, 2 insertions(+), 275 deletions(-)
 delete mode 100644 tools/testing/selftests/bpf/prog_tests/map_init.c
 delete mode 100644 tools/testing/selftests/bpf/progs/test_map_init.c

Comments

Kelsey Skunberg Dec. 5, 2020, 12:24 a.m. UTC | #1 
Verified getting the fatal error and it clearing after applying this
revert. lgtm. Thank you, Kamal. 

Acked-by: Kelsey Skunberg <kelsey.skunberg@canonical.com>

On 2020-12-04 10:47:57 , Kamal Mostafa wrote:
> BugLink: https://bugs.launchpad.net/bugs/1906866
> 
> This reverts commit d946d4ddd6af531398201e7ce8f73bd1d8a98e2a.
> 
> Reported upstream:
>  https://lore.kernel.org/stable/20201204182846.27110-1-kamal@canonical.com/T/#u
> 
> This v5.4.78 commit breaks the tools/testing/selftests/bpf build:
> 
> [linux-5.4.y] c602ad2b52dc bpf: Zero-fill re-used per-cpu map element
> [focal] d946d4ddd6af bpf: Zero-fill re-used per-cpu map element
> 
> Like this:
> 
>         prog_tests/map_init.c:5:10: fatal error: test_map_init.skel.h:
> No such file or directory
>             5 | #include "test_map_init.skel.h"
> 
> Because tools/testing/selftests/bpf/Makefile in v5.4 does not have the
> "skeleton header generation" stuff (circa v5.6).
> 
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> ---
>  kernel/bpf/hashtab.c                          |  30 +--
>  .../selftests/bpf/prog_tests/map_init.c       | 214 ------------------
>  .../selftests/bpf/progs/test_map_init.c       |  33 ---
>  3 files changed, 2 insertions(+), 275 deletions(-)
>  delete mode 100644 tools/testing/selftests/bpf/prog_tests/map_init.c
>  delete mode 100644 tools/testing/selftests/bpf/progs/test_map_init.c
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 03a67583f6fb..728ffec52cf3 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -709,32 +709,6 @@ static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
>  	}
>  }
>  
> -static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
> -			    void *value, bool onallcpus)
> -{
> -	/* When using prealloc and not setting the initial value on all cpus,
> -	 * zero-fill element values for other cpus (just as what happens when
> -	 * not using prealloc). Otherwise, bpf program has no way to ensure
> -	 * known initial values for cpus other than current one
> -	 * (onallcpus=false always when coming from bpf prog).
> -	 */
> -	if (htab_is_prealloc(htab) && !onallcpus) {
> -		u32 size = round_up(htab->map.value_size, 8);
> -		int current_cpu = raw_smp_processor_id();
> -		int cpu;
> -
> -		for_each_possible_cpu(cpu) {
> -			if (cpu == current_cpu)
> -				bpf_long_memcpy(per_cpu_ptr(pptr, cpu), value,
> -						size);
> -			else
> -				memset(per_cpu_ptr(pptr, cpu), 0, size);
> -		}
> -	} else {
> -		pcpu_copy_value(htab, pptr, value, onallcpus);
> -	}
> -}
> -
>  static bool fd_htab_map_needs_adjust(const struct bpf_htab *htab)
>  {
>  	return htab->map.map_type == BPF_MAP_TYPE_HASH_OF_MAPS &&
> @@ -805,7 +779,7 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
>  			}
>  		}
>  
> -		pcpu_init_value(htab, pptr, value, onallcpus);
> +		pcpu_copy_value(htab, pptr, value, onallcpus);
>  
>  		if (!prealloc)
>  			htab_elem_set_ptr(l_new, key_size, pptr);
> @@ -1101,7 +1075,7 @@ static int __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
>  		pcpu_copy_value(htab, htab_elem_get_ptr(l_old, key_size),
>  				value, onallcpus);
>  	} else {
> -		pcpu_init_value(htab, htab_elem_get_ptr(l_new, key_size),
> +		pcpu_copy_value(htab, htab_elem_get_ptr(l_new, key_size),
>  				value, onallcpus);
>  		hlist_nulls_add_head_rcu(&l_new->hash_node, head);
>  		l_new = NULL;
> diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
> deleted file mode 100644
> index 14a31109dd0e..000000000000
> --- a/tools/testing/selftests/bpf/prog_tests/map_init.c
> +++ /dev/null
> @@ -1,214 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
> -
> -#include <test_progs.h>
> -#include "test_map_init.skel.h"
> -
> -#define TEST_VALUE 0x1234
> -#define FILL_VALUE 0xdeadbeef
> -
> -static int nr_cpus;
> -static int duration;
> -
> -typedef unsigned long long map_key_t;
> -typedef unsigned long long map_value_t;
> -typedef struct {
> -	map_value_t v; /* padding */
> -} __bpf_percpu_val_align pcpu_map_value_t;
> -
> -
> -static int map_populate(int map_fd, int num)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	int i, err;
> -	map_key_t key;
> -
> -	for (i = 0; i < nr_cpus; i++)
> -		bpf_percpu(value, i) = FILL_VALUE;
> -
> -	for (key = 1; key <= num; key++) {
> -		err = bpf_map_update_elem(map_fd, &key, value, BPF_NOEXIST);
> -		if (!ASSERT_OK(err, "bpf_map_update_elem"))
> -			return -1;
> -	}
> -
> -	return 0;
> -}
> -
> -static struct test_map_init *setup(enum bpf_map_type map_type, int map_sz,
> -			    int *map_fd, int populate)
> -{
> -	struct test_map_init *skel;
> -	int err;
> -
> -	skel = test_map_init__open();
> -	if (!ASSERT_OK_PTR(skel, "skel_open"))
> -		return NULL;
> -
> -	err = bpf_map__set_type(skel->maps.hashmap1, map_type);
> -	if (!ASSERT_OK(err, "bpf_map__set_type"))
> -		goto error;
> -
> -	err = bpf_map__set_max_entries(skel->maps.hashmap1, map_sz);
> -	if (!ASSERT_OK(err, "bpf_map__set_max_entries"))
> -		goto error;
> -
> -	err = test_map_init__load(skel);
> -	if (!ASSERT_OK(err, "skel_load"))
> -		goto error;
> -
> -	*map_fd = bpf_map__fd(skel->maps.hashmap1);
> -	if (CHECK(*map_fd < 0, "bpf_map__fd", "failed\n"))
> -		goto error;
> -
> -	err = map_populate(*map_fd, populate);
> -	if (!ASSERT_OK(err, "map_populate"))
> -		goto error_map;
> -
> -	return skel;
> -
> -error_map:
> -	close(*map_fd);
> -error:
> -	test_map_init__destroy(skel);
> -	return NULL;
> -}
> -
> -/* executes bpf program that updates map with key, value */
> -static int prog_run_insert_elem(struct test_map_init *skel, map_key_t key,
> -				map_value_t value)
> -{
> -	struct test_map_init__bss *bss;
> -
> -	bss = skel->bss;
> -
> -	bss->inKey = key;
> -	bss->inValue = value;
> -	bss->inPid = getpid();
> -
> -	if (!ASSERT_OK(test_map_init__attach(skel), "skel_attach"))
> -		return -1;
> -
> -	/* Let tracepoint trigger */
> -	syscall(__NR_getpgid);
> -
> -	test_map_init__detach(skel);
> -
> -	return 0;
> -}
> -
> -static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
> -{
> -	int i, nzCnt = 0;
> -	map_value_t val;
> -
> -	for (i = 0; i < nr_cpus; i++) {
> -		val = bpf_percpu(value, i);
> -		if (val) {
> -			if (CHECK(val != expected, "map value",
> -				  "unexpected for cpu %d: 0x%llx\n", i, val))
> -				return -1;
> -			nzCnt++;
> -		}
> -	}
> -
> -	if (CHECK(nzCnt != 1, "map value", "set for %d CPUs instead of 1!\n",
> -		  nzCnt))
> -		return -1;
> -
> -	return 0;
> -}
> -
> -/* Add key=1 elem with values set for all CPUs
> - * Delete elem key=1
> - * Run bpf prog that inserts new key=1 elem with value=0x1234
> - *   (bpf prog can only set value for current CPU)
> - * Lookup Key=1 and check value is as expected for all CPUs:
> - *   value set by bpf prog for one CPU, 0 for all others
> - */
> -static void test_pcpu_map_init(void)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	struct test_map_init *skel;
> -	int map_fd, err;
> -	map_key_t key;
> -
> -	/* max 1 elem in map so insertion is forced to reuse freed entry */
> -	skel = setup(BPF_MAP_TYPE_PERCPU_HASH, 1, &map_fd, 1);
> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
> -		return;
> -
> -	/* delete element so the entry can be re-used*/
> -	key = 1;
> -	err = bpf_map_delete_elem(map_fd, &key);
> -	if (!ASSERT_OK(err, "bpf_map_delete_elem"))
> -		goto cleanup;
> -
> -	/* run bpf prog that inserts new elem, re-using the slot just freed */
> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
> -		goto cleanup;
> -
> -	/* check that key=1 was re-created by bpf prog */
> -	err = bpf_map_lookup_elem(map_fd, &key, value);
> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> -		goto cleanup;
> -
> -	/* and has expected values */
> -	check_values_one_cpu(value, TEST_VALUE);
> -
> -cleanup:
> -	test_map_init__destroy(skel);
> -}
> -
> -/* Add key=1 and key=2 elems with values set for all CPUs
> - * Run bpf prog that inserts new key=3 elem
> - *   (only for current cpu; other cpus should have initial value = 0)
> - * Lookup Key=1 and check value is as expected for all CPUs
> - */
> -static void test_pcpu_lru_map_init(void)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	struct test_map_init *skel;
> -	int map_fd, err;
> -	map_key_t key;
> -
> -	/* Set up LRU map with 2 elements, values filled for all CPUs.
> -	 * With these 2 elements, the LRU map is full
> -	 */
> -	skel = setup(BPF_MAP_TYPE_LRU_PERCPU_HASH, 2, &map_fd, 2);
> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
> -		return;
> -
> -	/* run bpf prog that inserts new key=3 element, re-using LRU slot */
> -	key = 3;
> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
> -		goto cleanup;
> -
> -	/* check that key=3 replaced one of earlier elements */
> -	err = bpf_map_lookup_elem(map_fd, &key, value);
> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> -		goto cleanup;
> -
> -	/* and has expected values */
> -	check_values_one_cpu(value, TEST_VALUE);
> -
> -cleanup:
> -	test_map_init__destroy(skel);
> -}
> -
> -void test_map_init(void)
> -{
> -	nr_cpus = bpf_num_possible_cpus();
> -	if (nr_cpus <= 1) {
> -		printf("%s:SKIP: >1 cpu needed for this test\n", __func__);
> -		test__skip();
> -		return;
> -	}
> -
> -	if (test__start_subtest("pcpu_map_init"))
> -		test_pcpu_map_init();
> -	if (test__start_subtest("pcpu_lru_map_init"))
> -		test_pcpu_lru_map_init();
> -}
> diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
> deleted file mode 100644
> index c89d28ead673..000000000000
> --- a/tools/testing/selftests/bpf/progs/test_map_init.c
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
> -
> -#include "vmlinux.h"
> -#include <bpf/bpf_helpers.h>
> -
> -__u64 inKey = 0;
> -__u64 inValue = 0;
> -__u32 inPid = 0;
> -
> -struct {
> -	__uint(type, BPF_MAP_TYPE_PERCPU_HASH);
> -	__uint(max_entries, 2);
> -	__type(key, __u64);
> -	__type(value, __u64);
> -} hashmap1 SEC(".maps");
> -
> -
> -SEC("tp/syscalls/sys_enter_getpgid")
> -int sysenter_getpgid(const void *ctx)
> -{
> -	/* Just do it for once, when called from our own test prog. This
> -	 * ensures the map value is only updated for a single CPU.
> -	 */
> -	int cur_pid = bpf_get_current_pid_tgid() >> 32;
> -
> -	if (cur_pid == inPid)
> -		bpf_map_update_elem(&hashmap1, &inKey, &inValue, BPF_NOEXIST);
> -
> -	return 0;
> -}
> -
> -char _license[] SEC("license") = "GPL";
> -- 
> 2.17.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Kleber Sacilotto de Souza Dec. 7, 2020, 9:57 a.m. UTC | #2 
On 04.12.20 19:47, Kamal Mostafa wrote:
> BugLink: https://bugs.launchpad.net/bugs/1906866
> 
> This reverts commit d946d4ddd6af531398201e7ce8f73bd1d8a98e2a.
> 
> Reported upstream:
>   https://lore.kernel.org/stable/20201204182846.27110-1-kamal@canonical.com/T/#u
> 
> This v5.4.78 commit breaks the tools/testing/selftests/bpf build:
> 
> [linux-5.4.y] c602ad2b52dc bpf: Zero-fill re-used per-cpu map element
> [focal] d946d4ddd6af bpf: Zero-fill re-used per-cpu map element
> 
> Like this:
> 
>          prog_tests/map_init.c:5:10: fatal error: test_map_init.skel.h:
> No such file or directory
>              5 | #include "test_map_init.skel.h"
> 
> Because tools/testing/selftests/bpf/Makefile in v5.4 does not have the
> "skeleton header generation" stuff (circa v5.6).
> 
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>

Thanks Kamal for the quick fix.

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> ---
>   kernel/bpf/hashtab.c                          |  30 +--
>   .../selftests/bpf/prog_tests/map_init.c       | 214 ------------------
>   .../selftests/bpf/progs/test_map_init.c       |  33 ---
>   3 files changed, 2 insertions(+), 275 deletions(-)
>   delete mode 100644 tools/testing/selftests/bpf/prog_tests/map_init.c
>   delete mode 100644 tools/testing/selftests/bpf/progs/test_map_init.c
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 03a67583f6fb..728ffec52cf3 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -709,32 +709,6 @@ static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
>   	}
>   }
>   
> -static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
> -			    void *value, bool onallcpus)
> -{
> -	/* When using prealloc and not setting the initial value on all cpus,
> -	 * zero-fill element values for other cpus (just as what happens when
> -	 * not using prealloc). Otherwise, bpf program has no way to ensure
> -	 * known initial values for cpus other than current one
> -	 * (onallcpus=false always when coming from bpf prog).
> -	 */
> -	if (htab_is_prealloc(htab) && !onallcpus) {
> -		u32 size = round_up(htab->map.value_size, 8);
> -		int current_cpu = raw_smp_processor_id();
> -		int cpu;
> -
> -		for_each_possible_cpu(cpu) {
> -			if (cpu == current_cpu)
> -				bpf_long_memcpy(per_cpu_ptr(pptr, cpu), value,
> -						size);
> -			else
> -				memset(per_cpu_ptr(pptr, cpu), 0, size);
> -		}
> -	} else {
> -		pcpu_copy_value(htab, pptr, value, onallcpus);
> -	}
> -}
> -
>   static bool fd_htab_map_needs_adjust(const struct bpf_htab *htab)
>   {
>   	return htab->map.map_type == BPF_MAP_TYPE_HASH_OF_MAPS &&
> @@ -805,7 +779,7 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
>   			}
>   		}
>   
> -		pcpu_init_value(htab, pptr, value, onallcpus);
> +		pcpu_copy_value(htab, pptr, value, onallcpus);
>   
>   		if (!prealloc)
>   			htab_elem_set_ptr(l_new, key_size, pptr);
> @@ -1101,7 +1075,7 @@ static int __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
>   		pcpu_copy_value(htab, htab_elem_get_ptr(l_old, key_size),
>   				value, onallcpus);
>   	} else {
> -		pcpu_init_value(htab, htab_elem_get_ptr(l_new, key_size),
> +		pcpu_copy_value(htab, htab_elem_get_ptr(l_new, key_size),
>   				value, onallcpus);
>   		hlist_nulls_add_head_rcu(&l_new->hash_node, head);
>   		l_new = NULL;
> diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
> deleted file mode 100644
> index 14a31109dd0e..000000000000
> --- a/tools/testing/selftests/bpf/prog_tests/map_init.c
> +++ /dev/null
> @@ -1,214 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
> -
> -#include <test_progs.h>
> -#include "test_map_init.skel.h"
> -
> -#define TEST_VALUE 0x1234
> -#define FILL_VALUE 0xdeadbeef
> -
> -static int nr_cpus;
> -static int duration;
> -
> -typedef unsigned long long map_key_t;
> -typedef unsigned long long map_value_t;
> -typedef struct {
> -	map_value_t v; /* padding */
> -} __bpf_percpu_val_align pcpu_map_value_t;
> -
> -
> -static int map_populate(int map_fd, int num)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	int i, err;
> -	map_key_t key;
> -
> -	for (i = 0; i < nr_cpus; i++)
> -		bpf_percpu(value, i) = FILL_VALUE;
> -
> -	for (key = 1; key <= num; key++) {
> -		err = bpf_map_update_elem(map_fd, &key, value, BPF_NOEXIST);
> -		if (!ASSERT_OK(err, "bpf_map_update_elem"))
> -			return -1;
> -	}
> -
> -	return 0;
> -}
> -
> -static struct test_map_init *setup(enum bpf_map_type map_type, int map_sz,
> -			    int *map_fd, int populate)
> -{
> -	struct test_map_init *skel;
> -	int err;
> -
> -	skel = test_map_init__open();
> -	if (!ASSERT_OK_PTR(skel, "skel_open"))
> -		return NULL;
> -
> -	err = bpf_map__set_type(skel->maps.hashmap1, map_type);
> -	if (!ASSERT_OK(err, "bpf_map__set_type"))
> -		goto error;
> -
> -	err = bpf_map__set_max_entries(skel->maps.hashmap1, map_sz);
> -	if (!ASSERT_OK(err, "bpf_map__set_max_entries"))
> -		goto error;
> -
> -	err = test_map_init__load(skel);
> -	if (!ASSERT_OK(err, "skel_load"))
> -		goto error;
> -
> -	*map_fd = bpf_map__fd(skel->maps.hashmap1);
> -	if (CHECK(*map_fd < 0, "bpf_map__fd", "failed\n"))
> -		goto error;
> -
> -	err = map_populate(*map_fd, populate);
> -	if (!ASSERT_OK(err, "map_populate"))
> -		goto error_map;
> -
> -	return skel;
> -
> -error_map:
> -	close(*map_fd);
> -error:
> -	test_map_init__destroy(skel);
> -	return NULL;
> -}
> -
> -/* executes bpf program that updates map with key, value */
> -static int prog_run_insert_elem(struct test_map_init *skel, map_key_t key,
> -				map_value_t value)
> -{
> -	struct test_map_init__bss *bss;
> -
> -	bss = skel->bss;
> -
> -	bss->inKey = key;
> -	bss->inValue = value;
> -	bss->inPid = getpid();
> -
> -	if (!ASSERT_OK(test_map_init__attach(skel), "skel_attach"))
> -		return -1;
> -
> -	/* Let tracepoint trigger */
> -	syscall(__NR_getpgid);
> -
> -	test_map_init__detach(skel);
> -
> -	return 0;
> -}
> -
> -static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
> -{
> -	int i, nzCnt = 0;
> -	map_value_t val;
> -
> -	for (i = 0; i < nr_cpus; i++) {
> -		val = bpf_percpu(value, i);
> -		if (val) {
> -			if (CHECK(val != expected, "map value",
> -				  "unexpected for cpu %d: 0x%llx\n", i, val))
> -				return -1;
> -			nzCnt++;
> -		}
> -	}
> -
> -	if (CHECK(nzCnt != 1, "map value", "set for %d CPUs instead of 1!\n",
> -		  nzCnt))
> -		return -1;
> -
> -	return 0;
> -}
> -
> -/* Add key=1 elem with values set for all CPUs
> - * Delete elem key=1
> - * Run bpf prog that inserts new key=1 elem with value=0x1234
> - *   (bpf prog can only set value for current CPU)
> - * Lookup Key=1 and check value is as expected for all CPUs:
> - *   value set by bpf prog for one CPU, 0 for all others
> - */
> -static void test_pcpu_map_init(void)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	struct test_map_init *skel;
> -	int map_fd, err;
> -	map_key_t key;
> -
> -	/* max 1 elem in map so insertion is forced to reuse freed entry */
> -	skel = setup(BPF_MAP_TYPE_PERCPU_HASH, 1, &map_fd, 1);
> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
> -		return;
> -
> -	/* delete element so the entry can be re-used*/
> -	key = 1;
> -	err = bpf_map_delete_elem(map_fd, &key);
> -	if (!ASSERT_OK(err, "bpf_map_delete_elem"))
> -		goto cleanup;
> -
> -	/* run bpf prog that inserts new elem, re-using the slot just freed */
> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
> -		goto cleanup;
> -
> -	/* check that key=1 was re-created by bpf prog */
> -	err = bpf_map_lookup_elem(map_fd, &key, value);
> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> -		goto cleanup;
> -
> -	/* and has expected values */
> -	check_values_one_cpu(value, TEST_VALUE);
> -
> -cleanup:
> -	test_map_init__destroy(skel);
> -}
> -
> -/* Add key=1 and key=2 elems with values set for all CPUs
> - * Run bpf prog that inserts new key=3 elem
> - *   (only for current cpu; other cpus should have initial value = 0)
> - * Lookup Key=1 and check value is as expected for all CPUs
> - */
> -static void test_pcpu_lru_map_init(void)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	struct test_map_init *skel;
> -	int map_fd, err;
> -	map_key_t key;
> -
> -	/* Set up LRU map with 2 elements, values filled for all CPUs.
> -	 * With these 2 elements, the LRU map is full
> -	 */
> -	skel = setup(BPF_MAP_TYPE_LRU_PERCPU_HASH, 2, &map_fd, 2);
> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
> -		return;
> -
> -	/* run bpf prog that inserts new key=3 element, re-using LRU slot */
> -	key = 3;
> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
> -		goto cleanup;
> -
> -	/* check that key=3 replaced one of earlier elements */
> -	err = bpf_map_lookup_elem(map_fd, &key, value);
> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> -		goto cleanup;
> -
> -	/* and has expected values */
> -	check_values_one_cpu(value, TEST_VALUE);
> -
> -cleanup:
> -	test_map_init__destroy(skel);
> -}
> -
> -void test_map_init(void)
> -{
> -	nr_cpus = bpf_num_possible_cpus();
> -	if (nr_cpus <= 1) {
> -		printf("%s:SKIP: >1 cpu needed for this test\n", __func__);
> -		test__skip();
> -		return;
> -	}
> -
> -	if (test__start_subtest("pcpu_map_init"))
> -		test_pcpu_map_init();
> -	if (test__start_subtest("pcpu_lru_map_init"))
> -		test_pcpu_lru_map_init();
> -}
> diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
> deleted file mode 100644
> index c89d28ead673..000000000000
> --- a/tools/testing/selftests/bpf/progs/test_map_init.c
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
> -
> -#include "vmlinux.h"
> -#include <bpf/bpf_helpers.h>
> -
> -__u64 inKey = 0;
> -__u64 inValue = 0;
> -__u32 inPid = 0;
> -
> -struct {
> -	__uint(type, BPF_MAP_TYPE_PERCPU_HASH);
> -	__uint(max_entries, 2);
> -	__type(key, __u64);
> -	__type(value, __u64);
> -} hashmap1 SEC(".maps");
> -
> -
> -SEC("tp/syscalls/sys_enter_getpgid")
> -int sysenter_getpgid(const void *ctx)
> -{
> -	/* Just do it for once, when called from our own test prog. This
> -	 * ensures the map value is only updated for a single CPU.
> -	 */
> -	int cur_pid = bpf_get_current_pid_tgid() >> 32;
> -
> -	if (cur_pid == inPid)
> -		bpf_map_update_elem(&hashmap1, &inKey, &inValue, BPF_NOEXIST);
> -
> -	return 0;
> -}
> -
> -char _license[] SEC("license") = "GPL";
>
Thadeu Lima de Souza Cascardo Dec. 7, 2020, 12:24 p.m. UTC | #3 
On Fri, Dec 04, 2020 at 10:47:57AM -0800, Kamal Mostafa wrote:
> BugLink: https://bugs.launchpad.net/bugs/1906866
> 
> This reverts commit d946d4ddd6af531398201e7ce8f73bd1d8a98e2a.
> 
> Reported upstream:
>  https://lore.kernel.org/stable/20201204182846.27110-1-kamal@canonical.com/T/#u
> 
> This v5.4.78 commit breaks the tools/testing/selftests/bpf build:
> 
> [linux-5.4.y] c602ad2b52dc bpf: Zero-fill re-used per-cpu map element
> [focal] d946d4ddd6af bpf: Zero-fill re-used per-cpu map element
> 
> Like this:
> 
>         prog_tests/map_init.c:5:10: fatal error: test_map_init.skel.h:
> No such file or directory
>             5 | #include "test_map_init.skel.h"
> 
> Because tools/testing/selftests/bpf/Makefile in v5.4 does not have the
> "skeleton header generation" stuff (circa v5.6).
> 

Is this just because of the new test? Why would it cause any existing tests to
fail? It's unfortunate that the commit puts the kernel change and the new test
together, because the kernel change seems legit and desirable from what I could
see.

Cascardo.

> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> ---
>  kernel/bpf/hashtab.c                          |  30 +--
>  .../selftests/bpf/prog_tests/map_init.c       | 214 ------------------
>  .../selftests/bpf/progs/test_map_init.c       |  33 ---
>  3 files changed, 2 insertions(+), 275 deletions(-)
>  delete mode 100644 tools/testing/selftests/bpf/prog_tests/map_init.c
>  delete mode 100644 tools/testing/selftests/bpf/progs/test_map_init.c
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 03a67583f6fb..728ffec52cf3 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -709,32 +709,6 @@ static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
>  	}
>  }
>  
> -static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
> -			    void *value, bool onallcpus)
> -{
> -	/* When using prealloc and not setting the initial value on all cpus,
> -	 * zero-fill element values for other cpus (just as what happens when
> -	 * not using prealloc). Otherwise, bpf program has no way to ensure
> -	 * known initial values for cpus other than current one
> -	 * (onallcpus=false always when coming from bpf prog).
> -	 */
> -	if (htab_is_prealloc(htab) && !onallcpus) {
> -		u32 size = round_up(htab->map.value_size, 8);
> -		int current_cpu = raw_smp_processor_id();
> -		int cpu;
> -
> -		for_each_possible_cpu(cpu) {
> -			if (cpu == current_cpu)
> -				bpf_long_memcpy(per_cpu_ptr(pptr, cpu), value,
> -						size);
> -			else
> -				memset(per_cpu_ptr(pptr, cpu), 0, size);
> -		}
> -	} else {
> -		pcpu_copy_value(htab, pptr, value, onallcpus);
> -	}
> -}
> -
>  static bool fd_htab_map_needs_adjust(const struct bpf_htab *htab)
>  {
>  	return htab->map.map_type == BPF_MAP_TYPE_HASH_OF_MAPS &&
> @@ -805,7 +779,7 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
>  			}
>  		}
>  
> -		pcpu_init_value(htab, pptr, value, onallcpus);
> +		pcpu_copy_value(htab, pptr, value, onallcpus);
>  
>  		if (!prealloc)
>  			htab_elem_set_ptr(l_new, key_size, pptr);
> @@ -1101,7 +1075,7 @@ static int __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
>  		pcpu_copy_value(htab, htab_elem_get_ptr(l_old, key_size),
>  				value, onallcpus);
>  	} else {
> -		pcpu_init_value(htab, htab_elem_get_ptr(l_new, key_size),
> +		pcpu_copy_value(htab, htab_elem_get_ptr(l_new, key_size),
>  				value, onallcpus);
>  		hlist_nulls_add_head_rcu(&l_new->hash_node, head);
>  		l_new = NULL;
> diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
> deleted file mode 100644
> index 14a31109dd0e..000000000000
> --- a/tools/testing/selftests/bpf/prog_tests/map_init.c
> +++ /dev/null
> @@ -1,214 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
> -
> -#include <test_progs.h>
> -#include "test_map_init.skel.h"
> -
> -#define TEST_VALUE 0x1234
> -#define FILL_VALUE 0xdeadbeef
> -
> -static int nr_cpus;
> -static int duration;
> -
> -typedef unsigned long long map_key_t;
> -typedef unsigned long long map_value_t;
> -typedef struct {
> -	map_value_t v; /* padding */
> -} __bpf_percpu_val_align pcpu_map_value_t;
> -
> -
> -static int map_populate(int map_fd, int num)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	int i, err;
> -	map_key_t key;
> -
> -	for (i = 0; i < nr_cpus; i++)
> -		bpf_percpu(value, i) = FILL_VALUE;
> -
> -	for (key = 1; key <= num; key++) {
> -		err = bpf_map_update_elem(map_fd, &key, value, BPF_NOEXIST);
> -		if (!ASSERT_OK(err, "bpf_map_update_elem"))
> -			return -1;
> -	}
> -
> -	return 0;
> -}
> -
> -static struct test_map_init *setup(enum bpf_map_type map_type, int map_sz,
> -			    int *map_fd, int populate)
> -{
> -	struct test_map_init *skel;
> -	int err;
> -
> -	skel = test_map_init__open();
> -	if (!ASSERT_OK_PTR(skel, "skel_open"))
> -		return NULL;
> -
> -	err = bpf_map__set_type(skel->maps.hashmap1, map_type);
> -	if (!ASSERT_OK(err, "bpf_map__set_type"))
> -		goto error;
> -
> -	err = bpf_map__set_max_entries(skel->maps.hashmap1, map_sz);
> -	if (!ASSERT_OK(err, "bpf_map__set_max_entries"))
> -		goto error;
> -
> -	err = test_map_init__load(skel);
> -	if (!ASSERT_OK(err, "skel_load"))
> -		goto error;
> -
> -	*map_fd = bpf_map__fd(skel->maps.hashmap1);
> -	if (CHECK(*map_fd < 0, "bpf_map__fd", "failed\n"))
> -		goto error;
> -
> -	err = map_populate(*map_fd, populate);
> -	if (!ASSERT_OK(err, "map_populate"))
> -		goto error_map;
> -
> -	return skel;
> -
> -error_map:
> -	close(*map_fd);
> -error:
> -	test_map_init__destroy(skel);
> -	return NULL;
> -}
> -
> -/* executes bpf program that updates map with key, value */
> -static int prog_run_insert_elem(struct test_map_init *skel, map_key_t key,
> -				map_value_t value)
> -{
> -	struct test_map_init__bss *bss;
> -
> -	bss = skel->bss;
> -
> -	bss->inKey = key;
> -	bss->inValue = value;
> -	bss->inPid = getpid();
> -
> -	if (!ASSERT_OK(test_map_init__attach(skel), "skel_attach"))
> -		return -1;
> -
> -	/* Let tracepoint trigger */
> -	syscall(__NR_getpgid);
> -
> -	test_map_init__detach(skel);
> -
> -	return 0;
> -}
> -
> -static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
> -{
> -	int i, nzCnt = 0;
> -	map_value_t val;
> -
> -	for (i = 0; i < nr_cpus; i++) {
> -		val = bpf_percpu(value, i);
> -		if (val) {
> -			if (CHECK(val != expected, "map value",
> -				  "unexpected for cpu %d: 0x%llx\n", i, val))
> -				return -1;
> -			nzCnt++;
> -		}
> -	}
> -
> -	if (CHECK(nzCnt != 1, "map value", "set for %d CPUs instead of 1!\n",
> -		  nzCnt))
> -		return -1;
> -
> -	return 0;
> -}
> -
> -/* Add key=1 elem with values set for all CPUs
> - * Delete elem key=1
> - * Run bpf prog that inserts new key=1 elem with value=0x1234
> - *   (bpf prog can only set value for current CPU)
> - * Lookup Key=1 and check value is as expected for all CPUs:
> - *   value set by bpf prog for one CPU, 0 for all others
> - */
> -static void test_pcpu_map_init(void)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	struct test_map_init *skel;
> -	int map_fd, err;
> -	map_key_t key;
> -
> -	/* max 1 elem in map so insertion is forced to reuse freed entry */
> -	skel = setup(BPF_MAP_TYPE_PERCPU_HASH, 1, &map_fd, 1);
> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
> -		return;
> -
> -	/* delete element so the entry can be re-used*/
> -	key = 1;
> -	err = bpf_map_delete_elem(map_fd, &key);
> -	if (!ASSERT_OK(err, "bpf_map_delete_elem"))
> -		goto cleanup;
> -
> -	/* run bpf prog that inserts new elem, re-using the slot just freed */
> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
> -		goto cleanup;
> -
> -	/* check that key=1 was re-created by bpf prog */
> -	err = bpf_map_lookup_elem(map_fd, &key, value);
> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> -		goto cleanup;
> -
> -	/* and has expected values */
> -	check_values_one_cpu(value, TEST_VALUE);
> -
> -cleanup:
> -	test_map_init__destroy(skel);
> -}
> -
> -/* Add key=1 and key=2 elems with values set for all CPUs
> - * Run bpf prog that inserts new key=3 elem
> - *   (only for current cpu; other cpus should have initial value = 0)
> - * Lookup Key=1 and check value is as expected for all CPUs
> - */
> -static void test_pcpu_lru_map_init(void)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	struct test_map_init *skel;
> -	int map_fd, err;
> -	map_key_t key;
> -
> -	/* Set up LRU map with 2 elements, values filled for all CPUs.
> -	 * With these 2 elements, the LRU map is full
> -	 */
> -	skel = setup(BPF_MAP_TYPE_LRU_PERCPU_HASH, 2, &map_fd, 2);
> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
> -		return;
> -
> -	/* run bpf prog that inserts new key=3 element, re-using LRU slot */
> -	key = 3;
> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
> -		goto cleanup;
> -
> -	/* check that key=3 replaced one of earlier elements */
> -	err = bpf_map_lookup_elem(map_fd, &key, value);
> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> -		goto cleanup;
> -
> -	/* and has expected values */
> -	check_values_one_cpu(value, TEST_VALUE);
> -
> -cleanup:
> -	test_map_init__destroy(skel);
> -}
> -
> -void test_map_init(void)
> -{
> -	nr_cpus = bpf_num_possible_cpus();
> -	if (nr_cpus <= 1) {
> -		printf("%s:SKIP: >1 cpu needed for this test\n", __func__);
> -		test__skip();
> -		return;
> -	}
> -
> -	if (test__start_subtest("pcpu_map_init"))
> -		test_pcpu_map_init();
> -	if (test__start_subtest("pcpu_lru_map_init"))
> -		test_pcpu_lru_map_init();
> -}
> diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
> deleted file mode 100644
> index c89d28ead673..000000000000
> --- a/tools/testing/selftests/bpf/progs/test_map_init.c
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
> -
> -#include "vmlinux.h"
> -#include <bpf/bpf_helpers.h>
> -
> -__u64 inKey = 0;
> -__u64 inValue = 0;
> -__u32 inPid = 0;
> -
> -struct {
> -	__uint(type, BPF_MAP_TYPE_PERCPU_HASH);
> -	__uint(max_entries, 2);
> -	__type(key, __u64);
> -	__type(value, __u64);
> -} hashmap1 SEC(".maps");
> -
> -
> -SEC("tp/syscalls/sys_enter_getpgid")
> -int sysenter_getpgid(const void *ctx)
> -{
> -	/* Just do it for once, when called from our own test prog. This
> -	 * ensures the map value is only updated for a single CPU.
> -	 */
> -	int cur_pid = bpf_get_current_pid_tgid() >> 32;
> -
> -	if (cur_pid == inPid)
> -		bpf_map_update_elem(&hashmap1, &inKey, &inValue, BPF_NOEXIST);
> -
> -	return 0;
> -}
> -
> -char _license[] SEC("license") = "GPL";
> -- 
> 2.17.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Kleber Sacilotto de Souza Dec. 7, 2020, 1:50 p.m. UTC | #4 
On 07.12.20 13:24, Thadeu Lima de Souza Cascardo wrote:
> On Fri, Dec 04, 2020 at 10:47:57AM -0800, Kamal Mostafa wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1906866
>>
>> This reverts commit d946d4ddd6af531398201e7ce8f73bd1d8a98e2a.
>>
>> Reported upstream:
>>   https://lore.kernel.org/stable/20201204182846.27110-1-kamal@canonical.com/T/#u
>>
>> This v5.4.78 commit breaks the tools/testing/selftests/bpf build:
>>
>> [linux-5.4.y] c602ad2b52dc bpf: Zero-fill re-used per-cpu map element
>> [focal] d946d4ddd6af bpf: Zero-fill re-used per-cpu map element
>>
>> Like this:
>>
>>          prog_tests/map_init.c:5:10: fatal error: test_map_init.skel.h:
>> No such file or directory
>>              5 | #include "test_map_init.skel.h"
>>
>> Because tools/testing/selftests/bpf/Makefile in v5.4 does not have the
>> "skeleton header generation" stuff (circa v5.6).
>>
> 
> Is this just because of the new test? Why would it cause any existing tests to
> fail? It's unfortunate that the commit puts the kernel change and the new test
> together, because the kernel change seems legit and desirable from what I could
> see.

It breaks the build of bpf selftests, which is also needed for the net tests.

> 
> Cascardo.
> 
>> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
>> ---
>>   kernel/bpf/hashtab.c                          |  30 +--
>>   .../selftests/bpf/prog_tests/map_init.c       | 214 ------------------
>>   .../selftests/bpf/progs/test_map_init.c       |  33 ---
>>   3 files changed, 2 insertions(+), 275 deletions(-)
>>   delete mode 100644 tools/testing/selftests/bpf/prog_tests/map_init.c
>>   delete mode 100644 tools/testing/selftests/bpf/progs/test_map_init.c
>>
>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>> index 03a67583f6fb..728ffec52cf3 100644
>> --- a/kernel/bpf/hashtab.c
>> +++ b/kernel/bpf/hashtab.c
>> @@ -709,32 +709,6 @@ static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
>>   	}
>>   }
>>   
>> -static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
>> -			    void *value, bool onallcpus)
>> -{
>> -	/* When using prealloc and not setting the initial value on all cpus,
>> -	 * zero-fill element values for other cpus (just as what happens when
>> -	 * not using prealloc). Otherwise, bpf program has no way to ensure
>> -	 * known initial values for cpus other than current one
>> -	 * (onallcpus=false always when coming from bpf prog).
>> -	 */
>> -	if (htab_is_prealloc(htab) && !onallcpus) {
>> -		u32 size = round_up(htab->map.value_size, 8);
>> -		int current_cpu = raw_smp_processor_id();
>> -		int cpu;
>> -
>> -		for_each_possible_cpu(cpu) {
>> -			if (cpu == current_cpu)
>> -				bpf_long_memcpy(per_cpu_ptr(pptr, cpu), value,
>> -						size);
>> -			else
>> -				memset(per_cpu_ptr(pptr, cpu), 0, size);
>> -		}
>> -	} else {
>> -		pcpu_copy_value(htab, pptr, value, onallcpus);
>> -	}
>> -}
>> -
>>   static bool fd_htab_map_needs_adjust(const struct bpf_htab *htab)
>>   {
>>   	return htab->map.map_type == BPF_MAP_TYPE_HASH_OF_MAPS &&
>> @@ -805,7 +779,7 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
>>   			}
>>   		}
>>   
>> -		pcpu_init_value(htab, pptr, value, onallcpus);
>> +		pcpu_copy_value(htab, pptr, value, onallcpus);
>>   
>>   		if (!prealloc)
>>   			htab_elem_set_ptr(l_new, key_size, pptr);
>> @@ -1101,7 +1075,7 @@ static int __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
>>   		pcpu_copy_value(htab, htab_elem_get_ptr(l_old, key_size),
>>   				value, onallcpus);
>>   	} else {
>> -		pcpu_init_value(htab, htab_elem_get_ptr(l_new, key_size),
>> +		pcpu_copy_value(htab, htab_elem_get_ptr(l_new, key_size),
>>   				value, onallcpus);
>>   		hlist_nulls_add_head_rcu(&l_new->hash_node, head);
>>   		l_new = NULL;
>> diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
>> deleted file mode 100644
>> index 14a31109dd0e..000000000000
>> --- a/tools/testing/selftests/bpf/prog_tests/map_init.c
>> +++ /dev/null
>> @@ -1,214 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0-only
>> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
>> -
>> -#include <test_progs.h>
>> -#include "test_map_init.skel.h"
>> -
>> -#define TEST_VALUE 0x1234
>> -#define FILL_VALUE 0xdeadbeef
>> -
>> -static int nr_cpus;
>> -static int duration;
>> -
>> -typedef unsigned long long map_key_t;
>> -typedef unsigned long long map_value_t;
>> -typedef struct {
>> -	map_value_t v; /* padding */
>> -} __bpf_percpu_val_align pcpu_map_value_t;
>> -
>> -
>> -static int map_populate(int map_fd, int num)
>> -{
>> -	pcpu_map_value_t value[nr_cpus];
>> -	int i, err;
>> -	map_key_t key;
>> -
>> -	for (i = 0; i < nr_cpus; i++)
>> -		bpf_percpu(value, i) = FILL_VALUE;
>> -
>> -	for (key = 1; key <= num; key++) {
>> -		err = bpf_map_update_elem(map_fd, &key, value, BPF_NOEXIST);
>> -		if (!ASSERT_OK(err, "bpf_map_update_elem"))
>> -			return -1;
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>> -static struct test_map_init *setup(enum bpf_map_type map_type, int map_sz,
>> -			    int *map_fd, int populate)
>> -{
>> -	struct test_map_init *skel;
>> -	int err;
>> -
>> -	skel = test_map_init__open();
>> -	if (!ASSERT_OK_PTR(skel, "skel_open"))
>> -		return NULL;
>> -
>> -	err = bpf_map__set_type(skel->maps.hashmap1, map_type);
>> -	if (!ASSERT_OK(err, "bpf_map__set_type"))
>> -		goto error;
>> -
>> -	err = bpf_map__set_max_entries(skel->maps.hashmap1, map_sz);
>> -	if (!ASSERT_OK(err, "bpf_map__set_max_entries"))
>> -		goto error;
>> -
>> -	err = test_map_init__load(skel);
>> -	if (!ASSERT_OK(err, "skel_load"))
>> -		goto error;
>> -
>> -	*map_fd = bpf_map__fd(skel->maps.hashmap1);
>> -	if (CHECK(*map_fd < 0, "bpf_map__fd", "failed\n"))
>> -		goto error;
>> -
>> -	err = map_populate(*map_fd, populate);
>> -	if (!ASSERT_OK(err, "map_populate"))
>> -		goto error_map;
>> -
>> -	return skel;
>> -
>> -error_map:
>> -	close(*map_fd);
>> -error:
>> -	test_map_init__destroy(skel);
>> -	return NULL;
>> -}
>> -
>> -/* executes bpf program that updates map with key, value */
>> -static int prog_run_insert_elem(struct test_map_init *skel, map_key_t key,
>> -				map_value_t value)
>> -{
>> -	struct test_map_init__bss *bss;
>> -
>> -	bss = skel->bss;
>> -
>> -	bss->inKey = key;
>> -	bss->inValue = value;
>> -	bss->inPid = getpid();
>> -
>> -	if (!ASSERT_OK(test_map_init__attach(skel), "skel_attach"))
>> -		return -1;
>> -
>> -	/* Let tracepoint trigger */
>> -	syscall(__NR_getpgid);
>> -
>> -	test_map_init__detach(skel);
>> -
>> -	return 0;
>> -}
>> -
>> -static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
>> -{
>> -	int i, nzCnt = 0;
>> -	map_value_t val;
>> -
>> -	for (i = 0; i < nr_cpus; i++) {
>> -		val = bpf_percpu(value, i);
>> -		if (val) {
>> -			if (CHECK(val != expected, "map value",
>> -				  "unexpected for cpu %d: 0x%llx\n", i, val))
>> -				return -1;
>> -			nzCnt++;
>> -		}
>> -	}
>> -
>> -	if (CHECK(nzCnt != 1, "map value", "set for %d CPUs instead of 1!\n",
>> -		  nzCnt))
>> -		return -1;
>> -
>> -	return 0;
>> -}
>> -
>> -/* Add key=1 elem with values set for all CPUs
>> - * Delete elem key=1
>> - * Run bpf prog that inserts new key=1 elem with value=0x1234
>> - *   (bpf prog can only set value for current CPU)
>> - * Lookup Key=1 and check value is as expected for all CPUs:
>> - *   value set by bpf prog for one CPU, 0 for all others
>> - */
>> -static void test_pcpu_map_init(void)
>> -{
>> -	pcpu_map_value_t value[nr_cpus];
>> -	struct test_map_init *skel;
>> -	int map_fd, err;
>> -	map_key_t key;
>> -
>> -	/* max 1 elem in map so insertion is forced to reuse freed entry */
>> -	skel = setup(BPF_MAP_TYPE_PERCPU_HASH, 1, &map_fd, 1);
>> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
>> -		return;
>> -
>> -	/* delete element so the entry can be re-used*/
>> -	key = 1;
>> -	err = bpf_map_delete_elem(map_fd, &key);
>> -	if (!ASSERT_OK(err, "bpf_map_delete_elem"))
>> -		goto cleanup;
>> -
>> -	/* run bpf prog that inserts new elem, re-using the slot just freed */
>> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
>> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
>> -		goto cleanup;
>> -
>> -	/* check that key=1 was re-created by bpf prog */
>> -	err = bpf_map_lookup_elem(map_fd, &key, value);
>> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
>> -		goto cleanup;
>> -
>> -	/* and has expected values */
>> -	check_values_one_cpu(value, TEST_VALUE);
>> -
>> -cleanup:
>> -	test_map_init__destroy(skel);
>> -}
>> -
>> -/* Add key=1 and key=2 elems with values set for all CPUs
>> - * Run bpf prog that inserts new key=3 elem
>> - *   (only for current cpu; other cpus should have initial value = 0)
>> - * Lookup Key=1 and check value is as expected for all CPUs
>> - */
>> -static void test_pcpu_lru_map_init(void)
>> -{
>> -	pcpu_map_value_t value[nr_cpus];
>> -	struct test_map_init *skel;
>> -	int map_fd, err;
>> -	map_key_t key;
>> -
>> -	/* Set up LRU map with 2 elements, values filled for all CPUs.
>> -	 * With these 2 elements, the LRU map is full
>> -	 */
>> -	skel = setup(BPF_MAP_TYPE_LRU_PERCPU_HASH, 2, &map_fd, 2);
>> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
>> -		return;
>> -
>> -	/* run bpf prog that inserts new key=3 element, re-using LRU slot */
>> -	key = 3;
>> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
>> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
>> -		goto cleanup;
>> -
>> -	/* check that key=3 replaced one of earlier elements */
>> -	err = bpf_map_lookup_elem(map_fd, &key, value);
>> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
>> -		goto cleanup;
>> -
>> -	/* and has expected values */
>> -	check_values_one_cpu(value, TEST_VALUE);
>> -
>> -cleanup:
>> -	test_map_init__destroy(skel);
>> -}
>> -
>> -void test_map_init(void)
>> -{
>> -	nr_cpus = bpf_num_possible_cpus();
>> -	if (nr_cpus <= 1) {
>> -		printf("%s:SKIP: >1 cpu needed for this test\n", __func__);
>> -		test__skip();
>> -		return;
>> -	}
>> -
>> -	if (test__start_subtest("pcpu_map_init"))
>> -		test_pcpu_map_init();
>> -	if (test__start_subtest("pcpu_lru_map_init"))
>> -		test_pcpu_lru_map_init();
>> -}
>> diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
>> deleted file mode 100644
>> index c89d28ead673..000000000000
>> --- a/tools/testing/selftests/bpf/progs/test_map_init.c
>> +++ /dev/null
>> @@ -1,33 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0
>> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
>> -
>> -#include "vmlinux.h"
>> -#include <bpf/bpf_helpers.h>
>> -
>> -__u64 inKey = 0;
>> -__u64 inValue = 0;
>> -__u32 inPid = 0;
>> -
>> -struct {
>> -	__uint(type, BPF_MAP_TYPE_PERCPU_HASH);
>> -	__uint(max_entries, 2);
>> -	__type(key, __u64);
>> -	__type(value, __u64);
>> -} hashmap1 SEC(".maps");
>> -
>> -
>> -SEC("tp/syscalls/sys_enter_getpgid")
>> -int sysenter_getpgid(const void *ctx)
>> -{
>> -	/* Just do it for once, when called from our own test prog. This
>> -	 * ensures the map value is only updated for a single CPU.
>> -	 */
>> -	int cur_pid = bpf_get_current_pid_tgid() >> 32;
>> -
>> -	if (cur_pid == inPid)
>> -		bpf_map_update_elem(&hashmap1, &inKey, &inValue, BPF_NOEXIST);
>> -
>> -	return 0;
>> -}
>> -
>> -char _license[] SEC("license") = "GPL";
>> -- 
>> 2.17.1
>>
>>
>> -- 
>> kernel-team mailing list
>> kernel-team@lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Kamal Mostafa Dec. 8, 2020, 6:35 p.m. UTC | #5 
Formally NAK'ing this.  Instead, apply:

    [SRU][Focal][PATCH] UBUNTU: SAUCE: Revert selftests/ "bpf: Zero-fill
    re-used per-cpu map element"

    ^^ This one matches what upstream 5.4-stable is planning to do.

 -Kamal

On Fri, Dec 04, 2020 at 10:47:57AM -0800, Kamal Mostafa wrote:
> BugLink: https://bugs.launchpad.net/bugs/1906866
> 
> This reverts commit d946d4ddd6af531398201e7ce8f73bd1d8a98e2a.
> 
> Reported upstream:
>  https://lore.kernel.org/stable/20201204182846.27110-1-kamal@canonical.com/T/#u
> 
> This v5.4.78 commit breaks the tools/testing/selftests/bpf build:
> 
> [linux-5.4.y] c602ad2b52dc bpf: Zero-fill re-used per-cpu map element
> [focal] d946d4ddd6af bpf: Zero-fill re-used per-cpu map element
> 
> Like this:
> 
>         prog_tests/map_init.c:5:10: fatal error: test_map_init.skel.h:
> No such file or directory
>             5 | #include "test_map_init.skel.h"
> 
> Because tools/testing/selftests/bpf/Makefile in v5.4 does not have the
> "skeleton header generation" stuff (circa v5.6).
> 
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> ---
>  kernel/bpf/hashtab.c                          |  30 +--
>  .../selftests/bpf/prog_tests/map_init.c       | 214 ------------------
>  .../selftests/bpf/progs/test_map_init.c       |  33 ---
>  3 files changed, 2 insertions(+), 275 deletions(-)
>  delete mode 100644 tools/testing/selftests/bpf/prog_tests/map_init.c
>  delete mode 100644 tools/testing/selftests/bpf/progs/test_map_init.c
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 03a67583f6fb..728ffec52cf3 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -709,32 +709,6 @@ static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
>  	}
>  }
>  
> -static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
> -			    void *value, bool onallcpus)
> -{
> -	/* When using prealloc and not setting the initial value on all cpus,
> -	 * zero-fill element values for other cpus (just as what happens when
> -	 * not using prealloc). Otherwise, bpf program has no way to ensure
> -	 * known initial values for cpus other than current one
> -	 * (onallcpus=false always when coming from bpf prog).
> -	 */
> -	if (htab_is_prealloc(htab) && !onallcpus) {
> -		u32 size = round_up(htab->map.value_size, 8);
> -		int current_cpu = raw_smp_processor_id();
> -		int cpu;
> -
> -		for_each_possible_cpu(cpu) {
> -			if (cpu == current_cpu)
> -				bpf_long_memcpy(per_cpu_ptr(pptr, cpu), value,
> -						size);
> -			else
> -				memset(per_cpu_ptr(pptr, cpu), 0, size);
> -		}
> -	} else {
> -		pcpu_copy_value(htab, pptr, value, onallcpus);
> -	}
> -}
> -
>  static bool fd_htab_map_needs_adjust(const struct bpf_htab *htab)
>  {
>  	return htab->map.map_type == BPF_MAP_TYPE_HASH_OF_MAPS &&
> @@ -805,7 +779,7 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
>  			}
>  		}
>  
> -		pcpu_init_value(htab, pptr, value, onallcpus);
> +		pcpu_copy_value(htab, pptr, value, onallcpus);
>  
>  		if (!prealloc)
>  			htab_elem_set_ptr(l_new, key_size, pptr);
> @@ -1101,7 +1075,7 @@ static int __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
>  		pcpu_copy_value(htab, htab_elem_get_ptr(l_old, key_size),
>  				value, onallcpus);
>  	} else {
> -		pcpu_init_value(htab, htab_elem_get_ptr(l_new, key_size),
> +		pcpu_copy_value(htab, htab_elem_get_ptr(l_new, key_size),
>  				value, onallcpus);
>  		hlist_nulls_add_head_rcu(&l_new->hash_node, head);
>  		l_new = NULL;
> diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
> deleted file mode 100644
> index 14a31109dd0e..000000000000
> --- a/tools/testing/selftests/bpf/prog_tests/map_init.c
> +++ /dev/null
> @@ -1,214 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
> -
> -#include <test_progs.h>
> -#include "test_map_init.skel.h"
> -
> -#define TEST_VALUE 0x1234
> -#define FILL_VALUE 0xdeadbeef
> -
> -static int nr_cpus;
> -static int duration;
> -
> -typedef unsigned long long map_key_t;
> -typedef unsigned long long map_value_t;
> -typedef struct {
> -	map_value_t v; /* padding */
> -} __bpf_percpu_val_align pcpu_map_value_t;
> -
> -
> -static int map_populate(int map_fd, int num)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	int i, err;
> -	map_key_t key;
> -
> -	for (i = 0; i < nr_cpus; i++)
> -		bpf_percpu(value, i) = FILL_VALUE;
> -
> -	for (key = 1; key <= num; key++) {
> -		err = bpf_map_update_elem(map_fd, &key, value, BPF_NOEXIST);
> -		if (!ASSERT_OK(err, "bpf_map_update_elem"))
> -			return -1;
> -	}
> -
> -	return 0;
> -}
> -
> -static struct test_map_init *setup(enum bpf_map_type map_type, int map_sz,
> -			    int *map_fd, int populate)
> -{
> -	struct test_map_init *skel;
> -	int err;
> -
> -	skel = test_map_init__open();
> -	if (!ASSERT_OK_PTR(skel, "skel_open"))
> -		return NULL;
> -
> -	err = bpf_map__set_type(skel->maps.hashmap1, map_type);
> -	if (!ASSERT_OK(err, "bpf_map__set_type"))
> -		goto error;
> -
> -	err = bpf_map__set_max_entries(skel->maps.hashmap1, map_sz);
> -	if (!ASSERT_OK(err, "bpf_map__set_max_entries"))
> -		goto error;
> -
> -	err = test_map_init__load(skel);
> -	if (!ASSERT_OK(err, "skel_load"))
> -		goto error;
> -
> -	*map_fd = bpf_map__fd(skel->maps.hashmap1);
> -	if (CHECK(*map_fd < 0, "bpf_map__fd", "failed\n"))
> -		goto error;
> -
> -	err = map_populate(*map_fd, populate);
> -	if (!ASSERT_OK(err, "map_populate"))
> -		goto error_map;
> -
> -	return skel;
> -
> -error_map:
> -	close(*map_fd);
> -error:
> -	test_map_init__destroy(skel);
> -	return NULL;
> -}
> -
> -/* executes bpf program that updates map with key, value */
> -static int prog_run_insert_elem(struct test_map_init *skel, map_key_t key,
> -				map_value_t value)
> -{
> -	struct test_map_init__bss *bss;
> -
> -	bss = skel->bss;
> -
> -	bss->inKey = key;
> -	bss->inValue = value;
> -	bss->inPid = getpid();
> -
> -	if (!ASSERT_OK(test_map_init__attach(skel), "skel_attach"))
> -		return -1;
> -
> -	/* Let tracepoint trigger */
> -	syscall(__NR_getpgid);
> -
> -	test_map_init__detach(skel);
> -
> -	return 0;
> -}
> -
> -static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
> -{
> -	int i, nzCnt = 0;
> -	map_value_t val;
> -
> -	for (i = 0; i < nr_cpus; i++) {
> -		val = bpf_percpu(value, i);
> -		if (val) {
> -			if (CHECK(val != expected, "map value",
> -				  "unexpected for cpu %d: 0x%llx\n", i, val))
> -				return -1;
> -			nzCnt++;
> -		}
> -	}
> -
> -	if (CHECK(nzCnt != 1, "map value", "set for %d CPUs instead of 1!\n",
> -		  nzCnt))
> -		return -1;
> -
> -	return 0;
> -}
> -
> -/* Add key=1 elem with values set for all CPUs
> - * Delete elem key=1
> - * Run bpf prog that inserts new key=1 elem with value=0x1234
> - *   (bpf prog can only set value for current CPU)
> - * Lookup Key=1 and check value is as expected for all CPUs:
> - *   value set by bpf prog for one CPU, 0 for all others
> - */
> -static void test_pcpu_map_init(void)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	struct test_map_init *skel;
> -	int map_fd, err;
> -	map_key_t key;
> -
> -	/* max 1 elem in map so insertion is forced to reuse freed entry */
> -	skel = setup(BPF_MAP_TYPE_PERCPU_HASH, 1, &map_fd, 1);
> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
> -		return;
> -
> -	/* delete element so the entry can be re-used*/
> -	key = 1;
> -	err = bpf_map_delete_elem(map_fd, &key);
> -	if (!ASSERT_OK(err, "bpf_map_delete_elem"))
> -		goto cleanup;
> -
> -	/* run bpf prog that inserts new elem, re-using the slot just freed */
> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
> -		goto cleanup;
> -
> -	/* check that key=1 was re-created by bpf prog */
> -	err = bpf_map_lookup_elem(map_fd, &key, value);
> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> -		goto cleanup;
> -
> -	/* and has expected values */
> -	check_values_one_cpu(value, TEST_VALUE);
> -
> -cleanup:
> -	test_map_init__destroy(skel);
> -}
> -
> -/* Add key=1 and key=2 elems with values set for all CPUs
> - * Run bpf prog that inserts new key=3 elem
> - *   (only for current cpu; other cpus should have initial value = 0)
> - * Lookup Key=1 and check value is as expected for all CPUs
> - */
> -static void test_pcpu_lru_map_init(void)
> -{
> -	pcpu_map_value_t value[nr_cpus];
> -	struct test_map_init *skel;
> -	int map_fd, err;
> -	map_key_t key;
> -
> -	/* Set up LRU map with 2 elements, values filled for all CPUs.
> -	 * With these 2 elements, the LRU map is full
> -	 */
> -	skel = setup(BPF_MAP_TYPE_LRU_PERCPU_HASH, 2, &map_fd, 2);
> -	if (!ASSERT_OK_PTR(skel, "prog_setup"))
> -		return;
> -
> -	/* run bpf prog that inserts new key=3 element, re-using LRU slot */
> -	key = 3;
> -	err = prog_run_insert_elem(skel, key, TEST_VALUE);
> -	if (!ASSERT_OK(err, "prog_run_insert_elem"))
> -		goto cleanup;
> -
> -	/* check that key=3 replaced one of earlier elements */
> -	err = bpf_map_lookup_elem(map_fd, &key, value);
> -	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> -		goto cleanup;
> -
> -	/* and has expected values */
> -	check_values_one_cpu(value, TEST_VALUE);
> -
> -cleanup:
> -	test_map_init__destroy(skel);
> -}
> -
> -void test_map_init(void)
> -{
> -	nr_cpus = bpf_num_possible_cpus();
> -	if (nr_cpus <= 1) {
> -		printf("%s:SKIP: >1 cpu needed for this test\n", __func__);
> -		test__skip();
> -		return;
> -	}
> -
> -	if (test__start_subtest("pcpu_map_init"))
> -		test_pcpu_map_init();
> -	if (test__start_subtest("pcpu_lru_map_init"))
> -		test_pcpu_lru_map_init();
> -}
> diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
> deleted file mode 100644
> index c89d28ead673..000000000000
> --- a/tools/testing/selftests/bpf/progs/test_map_init.c
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
> -
> -#include "vmlinux.h"
> -#include <bpf/bpf_helpers.h>
> -
> -__u64 inKey = 0;
> -__u64 inValue = 0;
> -__u32 inPid = 0;
> -
> -struct {
> -	__uint(type, BPF_MAP_TYPE_PERCPU_HASH);
> -	__uint(max_entries, 2);
> -	__type(key, __u64);
> -	__type(value, __u64);
> -} hashmap1 SEC(".maps");
> -
> -
> -SEC("tp/syscalls/sys_enter_getpgid")
> -int sysenter_getpgid(const void *ctx)
> -{
> -	/* Just do it for once, when called from our own test prog. This
> -	 * ensures the map value is only updated for a single CPU.
> -	 */
> -	int cur_pid = bpf_get_current_pid_tgid() >> 32;
> -
> -	if (cur_pid == inPid)
> -		bpf_map_update_elem(&hashmap1, &inKey, &inValue, BPF_NOEXIST);
> -
> -	return 0;
> -}
> -
> -char _license[] SEC("license") = "GPL";
> -- 
> 2.17.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 03a67583f6fb..728ffec52cf3 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -709,32 +709,6 @@  static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
 	}
 }
 
-static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
-			    void *value, bool onallcpus)
-{
-	/* When using prealloc and not setting the initial value on all cpus,
-	 * zero-fill element values for other cpus (just as what happens when
-	 * not using prealloc). Otherwise, bpf program has no way to ensure
-	 * known initial values for cpus other than current one
-	 * (onallcpus=false always when coming from bpf prog).
-	 */
-	if (htab_is_prealloc(htab) && !onallcpus) {
-		u32 size = round_up(htab->map.value_size, 8);
-		int current_cpu = raw_smp_processor_id();
-		int cpu;
-
-		for_each_possible_cpu(cpu) {
-			if (cpu == current_cpu)
-				bpf_long_memcpy(per_cpu_ptr(pptr, cpu), value,
-						size);
-			else
-				memset(per_cpu_ptr(pptr, cpu), 0, size);
-		}
-	} else {
-		pcpu_copy_value(htab, pptr, value, onallcpus);
-	}
-}
-
 static bool fd_htab_map_needs_adjust(const struct bpf_htab *htab)
 {
 	return htab->map.map_type == BPF_MAP_TYPE_HASH_OF_MAPS &&
@@ -805,7 +779,7 @@  static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
 			}
 		}
 
-		pcpu_init_value(htab, pptr, value, onallcpus);
+		pcpu_copy_value(htab, pptr, value, onallcpus);
 
 		if (!prealloc)
 			htab_elem_set_ptr(l_new, key_size, pptr);
@@ -1101,7 +1075,7 @@  static int __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
 		pcpu_copy_value(htab, htab_elem_get_ptr(l_old, key_size),
 				value, onallcpus);
 	} else {
-		pcpu_init_value(htab, htab_elem_get_ptr(l_new, key_size),
+		pcpu_copy_value(htab, htab_elem_get_ptr(l_new, key_size),
 				value, onallcpus);
 		hlist_nulls_add_head_rcu(&l_new->hash_node, head);
 		l_new = NULL;
diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
deleted file mode 100644
index 14a31109dd0e..000000000000
--- a/tools/testing/selftests/bpf/prog_tests/map_init.c
+++ /dev/null
@@ -1,214 +0,0 @@ 
-// SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
-
-#include <test_progs.h>
-#include "test_map_init.skel.h"
-
-#define TEST_VALUE 0x1234
-#define FILL_VALUE 0xdeadbeef
-
-static int nr_cpus;
-static int duration;
-
-typedef unsigned long long map_key_t;
-typedef unsigned long long map_value_t;
-typedef struct {
-	map_value_t v; /* padding */
-} __bpf_percpu_val_align pcpu_map_value_t;
-
-
-static int map_populate(int map_fd, int num)
-{
-	pcpu_map_value_t value[nr_cpus];
-	int i, err;
-	map_key_t key;
-
-	for (i = 0; i < nr_cpus; i++)
-		bpf_percpu(value, i) = FILL_VALUE;
-
-	for (key = 1; key <= num; key++) {
-		err = bpf_map_update_elem(map_fd, &key, value, BPF_NOEXIST);
-		if (!ASSERT_OK(err, "bpf_map_update_elem"))
-			return -1;
-	}
-
-	return 0;
-}
-
-static struct test_map_init *setup(enum bpf_map_type map_type, int map_sz,
-			    int *map_fd, int populate)
-{
-	struct test_map_init *skel;
-	int err;
-
-	skel = test_map_init__open();
-	if (!ASSERT_OK_PTR(skel, "skel_open"))
-		return NULL;
-
-	err = bpf_map__set_type(skel->maps.hashmap1, map_type);
-	if (!ASSERT_OK(err, "bpf_map__set_type"))
-		goto error;
-
-	err = bpf_map__set_max_entries(skel->maps.hashmap1, map_sz);
-	if (!ASSERT_OK(err, "bpf_map__set_max_entries"))
-		goto error;
-
-	err = test_map_init__load(skel);
-	if (!ASSERT_OK(err, "skel_load"))
-		goto error;
-
-	*map_fd = bpf_map__fd(skel->maps.hashmap1);
-	if (CHECK(*map_fd < 0, "bpf_map__fd", "failed\n"))
-		goto error;
-
-	err = map_populate(*map_fd, populate);
-	if (!ASSERT_OK(err, "map_populate"))
-		goto error_map;
-
-	return skel;
-
-error_map:
-	close(*map_fd);
-error:
-	test_map_init__destroy(skel);
-	return NULL;
-}
-
-/* executes bpf program that updates map with key, value */
-static int prog_run_insert_elem(struct test_map_init *skel, map_key_t key,
-				map_value_t value)
-{
-	struct test_map_init__bss *bss;
-
-	bss = skel->bss;
-
-	bss->inKey = key;
-	bss->inValue = value;
-	bss->inPid = getpid();
-
-	if (!ASSERT_OK(test_map_init__attach(skel), "skel_attach"))
-		return -1;
-
-	/* Let tracepoint trigger */
-	syscall(__NR_getpgid);
-
-	test_map_init__detach(skel);
-
-	return 0;
-}
-
-static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
-{
-	int i, nzCnt = 0;
-	map_value_t val;
-
-	for (i = 0; i < nr_cpus; i++) {
-		val = bpf_percpu(value, i);
-		if (val) {
-			if (CHECK(val != expected, "map value",
-				  "unexpected for cpu %d: 0x%llx\n", i, val))
-				return -1;
-			nzCnt++;
-		}
-	}
-
-	if (CHECK(nzCnt != 1, "map value", "set for %d CPUs instead of 1!\n",
-		  nzCnt))
-		return -1;
-
-	return 0;
-}
-
-/* Add key=1 elem with values set for all CPUs
- * Delete elem key=1
- * Run bpf prog that inserts new key=1 elem with value=0x1234
- *   (bpf prog can only set value for current CPU)
- * Lookup Key=1 and check value is as expected for all CPUs:
- *   value set by bpf prog for one CPU, 0 for all others
- */
-static void test_pcpu_map_init(void)
-{
-	pcpu_map_value_t value[nr_cpus];
-	struct test_map_init *skel;
-	int map_fd, err;
-	map_key_t key;
-
-	/* max 1 elem in map so insertion is forced to reuse freed entry */
-	skel = setup(BPF_MAP_TYPE_PERCPU_HASH, 1, &map_fd, 1);
-	if (!ASSERT_OK_PTR(skel, "prog_setup"))
-		return;
-
-	/* delete element so the entry can be re-used*/
-	key = 1;
-	err = bpf_map_delete_elem(map_fd, &key);
-	if (!ASSERT_OK(err, "bpf_map_delete_elem"))
-		goto cleanup;
-
-	/* run bpf prog that inserts new elem, re-using the slot just freed */
-	err = prog_run_insert_elem(skel, key, TEST_VALUE);
-	if (!ASSERT_OK(err, "prog_run_insert_elem"))
-		goto cleanup;
-
-	/* check that key=1 was re-created by bpf prog */
-	err = bpf_map_lookup_elem(map_fd, &key, value);
-	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
-		goto cleanup;
-
-	/* and has expected values */
-	check_values_one_cpu(value, TEST_VALUE);
-
-cleanup:
-	test_map_init__destroy(skel);
-}
-
-/* Add key=1 and key=2 elems with values set for all CPUs
- * Run bpf prog that inserts new key=3 elem
- *   (only for current cpu; other cpus should have initial value = 0)
- * Lookup Key=1 and check value is as expected for all CPUs
- */
-static void test_pcpu_lru_map_init(void)
-{
-	pcpu_map_value_t value[nr_cpus];
-	struct test_map_init *skel;
-	int map_fd, err;
-	map_key_t key;
-
-	/* Set up LRU map with 2 elements, values filled for all CPUs.
-	 * With these 2 elements, the LRU map is full
-	 */
-	skel = setup(BPF_MAP_TYPE_LRU_PERCPU_HASH, 2, &map_fd, 2);
-	if (!ASSERT_OK_PTR(skel, "prog_setup"))
-		return;
-
-	/* run bpf prog that inserts new key=3 element, re-using LRU slot */
-	key = 3;
-	err = prog_run_insert_elem(skel, key, TEST_VALUE);
-	if (!ASSERT_OK(err, "prog_run_insert_elem"))
-		goto cleanup;
-
-	/* check that key=3 replaced one of earlier elements */
-	err = bpf_map_lookup_elem(map_fd, &key, value);
-	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
-		goto cleanup;
-
-	/* and has expected values */
-	check_values_one_cpu(value, TEST_VALUE);
-
-cleanup:
-	test_map_init__destroy(skel);
-}
-
-void test_map_init(void)
-{
-	nr_cpus = bpf_num_possible_cpus();
-	if (nr_cpus <= 1) {
-		printf("%s:SKIP: >1 cpu needed for this test\n", __func__);
-		test__skip();
-		return;
-	}
-
-	if (test__start_subtest("pcpu_map_init"))
-		test_pcpu_map_init();
-	if (test__start_subtest("pcpu_lru_map_init"))
-		test_pcpu_lru_map_init();
-}
diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
deleted file mode 100644
index c89d28ead673..000000000000
--- a/tools/testing/selftests/bpf/progs/test_map_init.c
+++ /dev/null
@@ -1,33 +0,0 @@ 
-// SPDX-License-Identifier: GPL-2.0
-/* Copyright (c) 2020 Tessares SA <http://www.tessares.net> */
-
-#include "vmlinux.h"
-#include <bpf/bpf_helpers.h>
-
-__u64 inKey = 0;
-__u64 inValue = 0;
-__u32 inPid = 0;
-
-struct {
-	__uint(type, BPF_MAP_TYPE_PERCPU_HASH);
-	__uint(max_entries, 2);
-	__type(key, __u64);
-	__type(value, __u64);
-} hashmap1 SEC(".maps");
-
-
-SEC("tp/syscalls/sys_enter_getpgid")
-int sysenter_getpgid(const void *ctx)
-{
-	/* Just do it for once, when called from our own test prog. This
-	 * ensures the map value is only updated for a single CPU.
-	 */
-	int cur_pid = bpf_get_current_pid_tgid() >> 32;
-
-	if (cur_pid == inPid)
-		bpf_map_update_elem(&hashmap1, &inKey, &inValue, BPF_NOEXIST);
-
-	return 0;
-}
-
-char _license[] SEC("license") = "GPL";