[v3,bpf-next,0/2] introduce BPF_F_PRESERVE_ELEMS

This set introduces BPF_F_PRESERVE_ELEMS to perf event array for better
sharing of perf event. By default, perf event array removes the perf event
when the map fd used to add the event is closed. With BPF_F_PRESERVE_ELEMS
set, however, the perf event will stay in the array until it is removed, or
the map is closed.

---
Changes v2 => v3:
1. Move perf_event_fd_array_map_free() to avoid unnecessary forward
   declaration. (Daniel)

Changes v1 => v2:
1. Rename the flag as BPF_F_PRESERVE_ELEMS. (Alexei, Daniel)
2. Simplify the code and selftest. (Daniel, Alexei)

Song Liu (2):
  bpf: introduce BPF_F_PRESERVE_ELEMS for perf event array
  selftests/bpf: add tests for BPF_F_PRESERVE_ELEMS

 include/uapi/linux/bpf.h                      |  3 +
 kernel/bpf/arraymap.c                         | 19 +++++-
 tools/include/uapi/linux/bpf.h                |  3 +
 .../bpf/prog_tests/pe_preserve_elems.c        | 66 +++++++++++++++++++
 .../bpf/progs/test_pe_preserve_elems.c        | 44 +++++++++++++
 5 files changed, 133 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/pe_preserve_elems.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c

--
2.24.1

On Wed, Sep 30, 2020 at 08:20:58AM -0700, Song Liu wrote:
> diff --git a/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c b/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c
> new file mode 100644
> index 0000000000000..dc77e406de41f
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2020 Facebook
> +#include "vmlinux.h"

Does it actually need vmlinux.h ?
Just checking to make sure it compiles on older kernels.

> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> +	__uint(max_entries, 1);
> +	__uint(key_size, sizeof(int));
> +	__uint(value_size, sizeof(int));
> +} array_1 SEC(".maps");
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> +	__uint(max_entries, 1);
> +	__uint(key_size, sizeof(int));
> +	__uint(value_size, sizeof(int));
> +	__uint(map_flags, BPF_F_PRESERVE_ELEMS);
> +} array_2 SEC(".maps");
> +
> +SEC("raw_tp/sched_switch")
> +int BPF_PROG(read_array_1)
> +{
> +	struct bpf_perf_event_value val;
> +	long ret;
> +
> +	ret = bpf_perf_event_read_value(&array_1, 0, &val, sizeof(val));
> +	bpf_printk("read_array_1 returns %ld", ret);
> +	return ret;
> +}
> +
> +SEC("raw_tp/task_rename")
> +int BPF_PROG(read_array_2)
> +{
> +	struct bpf_perf_event_value val;
> +	long ret;
> +
> +	ret = bpf_perf_event_read_value(&array_2, 0, &val, sizeof(val));
> +	bpf_printk("read_array_2 returns %ld", ret);

Please remove printk from the tests. It only spams the trace_pipe.

> +	return ret;

The return code is already checked as far as I can see.
That's enough to pass/fail the test, right?

> On Sep 30, 2020, at 12:26 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
> On Wed, Sep 30, 2020 at 08:20:58AM -0700, Song Liu wrote:
>> diff --git a/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c b/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c
>> new file mode 100644
>> index 0000000000000..dc77e406de41f
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c
>> @@ -0,0 +1,44 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) 2020 Facebook
>> +#include "vmlinux.h"
> 
> Does it actually need vmlinux.h ?
> Just checking to make sure it compiles on older kernels.

We can include linux/bpf.h instead. 

[...]

>> +	long ret;
>> +
>> +	ret = bpf_perf_event_read_value(&array_2, 0, &val, sizeof(val));
>> +	bpf_printk("read_array_2 returns %ld", ret);
> 
> Please remove printk from the tests. It only spams the trace_pipe.
> 
>> +	return ret;
> 
> The return code is already checked as far as I can see.
> That's enough to pass/fail the test, right?

Yes, we can remove the bpf_printk() here. Fixing this in v4.

Thanks,
Song