From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S932441AbbERStC (ORCPT <rfc822;w@1wt.eu>);
	Mon, 18 May 2015 14:49:02 -0400
Received: from mail-ie0-f178.google.com ([209.85.223.178]:34194 "EHLO
	mail-ie0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S932235AbbERStA (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 18 May 2015 14:49:00 -0400
Message-ID: <555A3419.4040207@plumgrid.com>
Date: Mon, 18 May 2015 11:48:57 -0700
From: Alexei Starovoitov <ast@plumgrid.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0
MIME-Version: 1.0
To: Wang Nan <wangnan0@huawei.com>, paulus@samba.org, a.p.zijlstra@chello.nl,
        mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
        jolsa@kernel.org, dsahern@gmail.com, daniel@iogearbox.net,
        brendan.d.gregg@gmail.com, masami.hiramatsu.pt@hitachi.com
CC: lizefan@huawei.com, linux-kernel@vger.kernel.org, pi3orama@163.com
Subject: Re: [RFC PATCH v3 21/37] bpf tools: Create eBPF maps defined in an
 object file
References: <1431860222-61636-1-git-send-email-wangnan0@huawei.com> <1431860222-61636-22-git-send-email-wangnan0@huawei.com>
In-Reply-To: <1431860222-61636-22-git-send-email-wangnan0@huawei.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 5/17/15 3:56 AM, Wang Nan wrote:
> This patch creates maps based on 'map' section in object file using
> bpf_create_map(), and store the fds into an array in
> 'struct bpf_object'. Since the byte order of the object may differ
> from the host, swap map definition before processing.
>
> This is the first patch in 'loading' phase. Previous patches parse ELF
> object file and create needed data structure, but doesnnt play with
> kernel. They belong to 'opening' phase.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
...
> +			for (j = 0; j < i; j++) {
> +				close(obj->maps_fds[j]);
> +				obj->maps_fds[j] = -1;

this line is unnecessary, since you're freeing the whole array
at the line below:

> +			}
> +			free(obj->maps_fds);
> +			obj->maps_fds = NULL;

...

>   void bpf_close_object(struct bpf_object *obj)
>   {
>   	if (!obj)
>   		return;
>
>   	bpf_obj_clear_elf(obj);
> +	bpf_unload_object(obj);

just realized that this API keeps elf file open for the whole
life time. I think that should be split up.
bpf_open_object() can do elf parsing, creation of maps and
bpf loading.
bpf_close_object() can unload programs, maps. That's fine,
but closing of elf file and freeing memory associated
with parsing sections should be a separate call.