Messages in this thread Patch in this message | | | Date | Tue, 7 Jul 2015 16:47:59 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [RFC PATCH v10 22/50] bpf tools: Link all bpf objects onto a list |
| |
Em Wed, Jul 01, 2015 at 02:14:10AM +0000, Wang Nan escreveu: > To prevent caller from creating additional structures to hold > pointers of 'struct bpf_object', this patch link all such
Can you rephrase the above sentence? You mean "to allow enumeration of all bpf_objects, keep them in a list".
> structures onto a list (hidden to user). bpf_object__for_each() is > introduced to allow users iterate over each objects. > bpf_object__for_each() is safe even user close the object during > iteration.
We have a idiom for that, it s called foo__for_each_entry_safe(), see list.h, tools/perf/util/evlist.h as well usess those same idioms, please keep that semantic.
I.e. if one isn't modifying the list while traversing it, no need for that tmp, if one is, the _safe() variant should be used, that requires the tmp.
We also have list_first_entry(), list_last_entry(), etc, its good to keep those semantics, helps people used to kernel (and tools/) programming to feel at home when reading your code :-)
- Arnaldo > Signed-off-by: Wang Nan <wangnan0@huawei.com> > Acked-by: Alexei Starovoitov <ast@plumgrid.com> > --- > tools/lib/bpf/libbpf.c | 32 ++++++++++++++++++++++++++++++++ > tools/lib/bpf/libbpf.h | 7 +++++++ > 2 files changed, 39 insertions(+) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 1c210fb..540ac558 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -17,6 +17,7 @@ > #include <asm/unistd.h> > #include <linux/kernel.h> > #include <linux/bpf.h> > +#include <linux/list.h> > #include <libelf.h> > #include <gelf.h> > > @@ -104,6 +105,8 @@ struct bpf_program { > bpf_program_clear_priv_t clear_priv; > }; > > +static LIST_HEAD(bpf_objects_list); > + > struct bpf_object { > char license[64]; > u32 kern_version; > @@ -137,6 +140,12 @@ struct bpf_object { > } *reloc; > int nr_reloc; > } efile; > + /* > + * All loaded bpf_object is linked in a list, which is > + * hidden to caller. bpf_objects__<func> handlers deal with > + * all objects. > + */ > + struct list_head list; > char path[]; > }; > #define obj_elf_valid(o) ((o)->efile.elf) > @@ -254,6 +263,9 @@ static struct bpf_object *bpf_object__new(const char *path, > obj->efile.obj_buf_sz = obj_buf_sz; > > obj->loaded = false; > + > + INIT_LIST_HEAD(&obj->list); > + list_add(&obj->list, &bpf_objects_list); > return obj; > } > > @@ -933,6 +945,7 @@ void bpf_object__close(struct bpf_object *obj) > } > zfree(&obj->programs); > > + list_del(&obj->list); > free(obj); > } > > @@ -945,6 +958,25 @@ int bpf_object__get_prog_cnt(struct bpf_object *obj, size_t *pcnt) > return 0; > } > > +struct bpf_object * > +bpf_object__next(struct bpf_object *prev) > +{ > + struct bpf_object *next; > + > + if (!prev) > + next = list_first_entry(&bpf_objects_list, > + struct bpf_object, > + list); > + else > + next = list_next_entry(prev, list); > + > + /* Empty list is noticed here so don't need checking on entry. */ > + if (&next->list == &bpf_objects_list) > + return NULL; > + > + return next; > +} > + > struct bpf_program * > bpf_program__next(struct bpf_program *prev, struct bpf_object *obj) > { > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index a20ae2e..1cd17a2 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -38,6 +38,13 @@ int bpf_object__unload(struct bpf_object *obj); > /* Accessors of bpf_object */ > int bpf_object__get_prog_cnt(struct bpf_object *obj, size_t *pcnt); > > +struct bpf_object *bpf_object__next(struct bpf_object *prev); > +#define bpf_object__for_each(pos, tmp) \ > + for ((pos) = bpf_object__next(NULL), \ > + (tmp) = bpf_object__next(pos); \ > + (pos) != NULL; \ > + (pos) = (tmp), (tmp) = bpf_object__next(tmp)) > + > /* Accessors of bpf_program. */ > struct bpf_program; > struct bpf_program *bpf_program__next(struct bpf_program *prog, > -- > 1.8.3.4
| |