On Tue, Sep 29, 2020 at 04:02:10PM +0200, Daniel Borkmann wrote:
> > +
> > +/* 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/ ?

> > +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

> > +}
> > +
> >   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.

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

Reply via email to