Re: [PATCH bpf-next 1/2] bpf: introduce BPF_F_SHARE_PE for perf event array

On 9/29/20 9:00 PM, Alexei Starovoitov wrote:

On Tue, Sep 29, 2020 at 04:02:10PM +0200, Daniel Borkmann wrote:

quoted

quoted

+
+/* Share perf_event among processes */
+	BPF_F_SHARE_PE		= (1U << 11),

nit but given UAPI: maybe name into something more self-descriptive
like BPF_F_SHAREABLE_EVENT ?

I'm not happy with either name.
It's not about sharing and not really about perf event.
I think the current behavior of perf_event_array is unusual and surprising.
Sadly we cannot fix it without breaking user space, so flag is needed.
How about BPF_F_STICKY_OBJECTS or BPF_F_PRESERVE_OBJECTS
or the same with s/OBJECTS/FILES/ ?

Sounds good to me, BPF_F_PRESERVE_OBJECTS or _ENTRIES seems reasonable.

quoted

quoted

+static void perf_event_fd_array_map_free(struct bpf_map *map)
+{
+	struct bpf_event_entry *ee;
+	struct bpf_array *array;
+	int i;
+
+	if ((map->map_flags & BPF_F_SHARE_PE) == 0) {
+		fd_array_map_free(map);
+		return;
+	}
+
+	array = container_of(map, struct bpf_array, map);
+	for (i = 0; i < array->map.max_entries; i++) {
+		ee = READ_ONCE(array->ptrs[i]);
+		if (ee)
+			fd_array_map_delete_elem(map, &i);
+	}
+	bpf_map_area_free(array);

Why not simplify into:

	if (map->map_flags & BPF_F_SHAREABLE_EVENT)
		bpf_fd_array_map_clear(map);
	fd_array_map_free(map);

+1

quoted

quoted

+}
+
   static void *prog_fd_array_get_ptr(struct bpf_map *map,
   				   struct file *map_file, int fd)
   {

@@ -1134,6 +1158,9 @@ static void perf_event_fd_array_release(struct bpf_map *map,
   	struct bpf_event_entry *ee;
   	int i;

add empty line pls.

quoted

quoted

+	if (map->map_flags & BPF_F_SHARE_PE)
+		return;
+