Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751963AbcDTTUs (ORCPT ); Wed, 20 Apr 2016 15:20:48 -0400 Received: from mail.kernel.org ([198.145.29.136]:33636 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750931AbcDTTUq (ORCPT ); Wed, 20 Apr 2016 15:20:46 -0400 Date: Wed, 20 Apr 2016 16:20:37 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: jolsa@redhat.com, brendan.d.gregg@gmail.com, linux-kernel@vger.kernel.org, pi3orama@163.com, Arnaldo Carvalho de Melo , Alexei Starovoitov , Jiri Olsa , Li Zefan Subject: Re: [RFC PATCH 04/13] bpf tools: Replace fd array to union array Message-ID: <20160420192037.GT3677@kernel.org> References: <1461175313-38310-1-git-send-email-wangnan0@huawei.com> <1461175313-38310-5-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1461175313-38310-5-git-send-email-wangnan0@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5927 Lines: 202 Em Wed, Apr 20, 2016 at 06:01:44PM +0000, Wang Nan escreveu: > Following commits will add new types for instance of a bpf program. > For kernel bpf program, a program instance is fd. For ubpf program, > the instance of a program is a 'struct ubpf_vm'. This patch promote > original 'fds' array to 'union prog_instance' array, prepare for > further extending. > > Signed-off-by: Wang Nan > Cc: Arnaldo Carvalho de Melo > Cc: Alexei Starovoitov > Cc: Brendan Gregg > Cc: Jiri Olsa > Cc: Li Zefan > --- > tools/lib/bpf/libbpf.c | 44 ++++++++++++++++++++++++-------------------- > 1 file changed, 24 insertions(+), 20 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 55b739e..dd5a107 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -138,6 +138,10 @@ int libbpf_strerror(int err, char *buf, size_t size) > # define LIBBPF_ELF_C_READ_MMAP ELF_C_READ > #endif > > +union prog_instance { > + int fd; > +}; > + > /* > * bpf_prog should be a better name but it has been used in > * linux/filter.h. > @@ -157,7 +161,7 @@ struct bpf_program { > > struct { > int nr; > - int *fds; > + union prog_instance *array; > } instances; Why not just do: struct { int nr; union { int *fds; }; } instances; And then do no further changes and when going to the ubpf (haven't looked at the actual patch, proposing this based on your above description), just transform it into: struct { int nr; union { int *fds; struct ubpf_vm *vms; }; } instances; Or plain use a void pointer and use: struct { int nr; void *entries; } instances; if (prog->instances.nr > 0) { int *fds = prog->instances.entries; for (i = 0; i < prog->instances.nr; i++) zclose(fd[i]); } > bpf_program_prep_t preprocessor; > > @@ -229,14 +233,14 @@ static void bpf_program__unload(struct bpf_program *prog) > */ > if (prog->instances.nr > 0) { > for (i = 0; i < prog->instances.nr; i++) > - zclose(prog->instances.fds[i]); > + zclose(prog->instances.array[i].fd); > } else if (prog->instances.nr != -1) { > pr_warning("Internal error: instances.nr is %d\n", > prog->instances.nr); > } > > prog->instances.nr = -1; > - zfree(&prog->instances.fds); > + zfree(&prog->instances.array); > } > > static void bpf_program__exit(struct bpf_program *prog) > @@ -287,7 +291,7 @@ bpf_program__init(void *data, size_t size, char *name, int idx, > memcpy(prog->insns, data, > prog->insns_cnt * sizeof(struct bpf_insn)); > prog->idx = idx; > - prog->instances.fds = NULL; > + prog->instances.array = NULL; > prog->instances.nr = -1; > > return 0; > @@ -937,20 +941,20 @@ bpf_program__load(struct bpf_program *prog, > { > int err = 0, fd, i; > > - if (prog->instances.nr < 0 || !prog->instances.fds) { > + if (prog->instances.nr < 0 || !prog->instances.array) { > if (prog->preprocessor) { > pr_warning("Internal error: can't load program '%s'\n", > prog->section_name); > return -LIBBPF_ERRNO__INTERNAL; > } > > - prog->instances.fds = malloc(sizeof(int)); > - if (!prog->instances.fds) { > - pr_warning("Not enough memory for BPF fds\n"); > + prog->instances.array = malloc(sizeof(union prog_instance)); > + if (!prog->instances.array) { > + pr_warning("Not enough memory for BPF instance\n"); > return -ENOMEM; > } > prog->instances.nr = 1; > - prog->instances.fds[0] = -1; > + prog->instances.array[0].fd = -1; > } > > if (!prog->preprocessor) { > @@ -961,7 +965,7 @@ bpf_program__load(struct bpf_program *prog, > err = load_program(prog->insns, prog->insns_cnt, > license, kern_version, &fd); > if (!err) > - prog->instances.fds[0] = fd; > + prog->instances.array[0].fd = fd; > goto out; > } > > @@ -981,7 +985,7 @@ bpf_program__load(struct bpf_program *prog, > if (!result.new_insn_ptr || !result.new_insn_cnt) { > pr_debug("Skip loading the %dth instance of program '%s'\n", > i, prog->section_name); > - prog->instances.fds[i] = -1; > + prog->instances.array[i].fd = -1; > if (result.pfd) > *result.pfd = -1; > continue; > @@ -999,7 +1003,7 @@ bpf_program__load(struct bpf_program *prog, > > if (result.pfd) > *result.pfd = fd; > - prog->instances.fds[i] = fd; > + prog->instances.array[i].fd = fd; > } > out: > if (err) > @@ -1269,27 +1273,27 @@ int bpf_program__fd(struct bpf_program *prog) > int bpf_program__set_prep(struct bpf_program *prog, int nr_instances, > bpf_program_prep_t prep) > { > - int *instances_fds; > + union prog_instance *array; > > if (nr_instances <= 0 || !prep) > return -EINVAL; > > - if (prog->instances.nr > 0 || prog->instances.fds) { > + if (prog->instances.nr > 0 || prog->instances.array) { > pr_warning("Can't set pre-processor after loading\n"); > return -EINVAL; > } > > - instances_fds = malloc(sizeof(int) * nr_instances); > - if (!instances_fds) { > - pr_warning("alloc memory failed for fds\n"); > + array = malloc(sizeof(array[0]) * nr_instances); > + if (!array) { > + pr_warning("alloc memory failed for instances\n"); > return -ENOMEM; > } > > /* fill all fd with -1 */ > - memset(instances_fds, -1, sizeof(int) * nr_instances); > + memset(array, -1, sizeof(array[0]) * nr_instances); > > prog->instances.nr = nr_instances; > - prog->instances.fds = instances_fds; > + prog->instances.array = array; > prog->preprocessor = prep; > return 0; > } > @@ -1304,7 +1308,7 @@ int bpf_program__nth_fd(struct bpf_program *prog, int n) > return -EINVAL; > } > > - fd = prog->instances.fds[n]; > + fd = prog->instances.array[n].fd; > if (fd < 0) { > pr_warning("%dth instance of program '%s' is invalid\n", > n, prog->section_name); > -- > 1.8.3.4