From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 3F7E48011 for ; Wed, 29 Oct 2014 11:17:50 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 29 Oct 2014 03:24:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,809,1406617200"; d="scan'208";a="598232127" Received: from bricha3-mobl.ger.corp.intel.com (HELO bricha3-mobl.ir.intel.com) ([10.243.20.19]) by orsmga001.jf.intel.com with SMTP; 29 Oct 2014 03:26:36 -0700 Received: by bricha3-mobl.ir.intel.com (sSMTP sendmail emulation); Wed, 29 Oct 2014 10:26:35 +0100 Date: Wed, 29 Oct 2014 10:26:35 +0000 From: Bruce Richardson To: Linhaifeng Message-ID: <20141029102635.GB8292@BRICHA3-MOBL> References: <1414551269-5820-1-git-send-email-haifeng.lin@huawei.com> <533710CFB86FA344BFBF2D6802E60286C7CAAB@SHSMSX101.ccr.corp.intel.com> <20141029034437.GA29486@mhcomputing.net> <533710CFB86FA344BFBF2D6802E60286C7CB42@SHSMSX101.ccr.corp.intel.com> <54508DE1.9090908@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <54508DE1.9090908@huawei.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] add free hugepage function X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Oct 2014 10:17:52 -0000 On Wed, Oct 29, 2014 at 02:49:05PM +0800, Linhaifeng wrote: > > > On 2014/10/29 13:26, Qiu, Michael wrote: > > 在 10/29/2014 11:46 AM, Matthew Hall 写道: > >> On Wed, Oct 29, 2014 at 03:27:58AM +0000, Qiu, Michael wrote: > >>> I just saw one return path with value '0', and no any other place > >>> return a negative value, so it is better to be designed as one > >>> non-return function, > >>> > >>> +void > >>> +rte_eal_hugepage_free(void) > >>> +{ > >>> + struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl; > >>> + unsigned i; > >>> + unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles; > >>> + > >>> + RTE_LOG(INFO, EAL, "unlink %u hugepage files\n", nr_hugefiles); > >>> + > >>> + for (i = 0; i < nr_hugefiles; i++) { > >>> + unlink(hugepg_tbl[i].filepath); > >>> + hugepg_tbl[i].orig_va = NULL; > >>> + } > >>> +} > >>> + > >>> > >>> Thanks, > >>> Michael > >> Actually, I don't think that's quite right. > >> > >> http://linux.die.net/man/2/unlink > >> > >> "On success, zero is returned. On error, -1 is returned, and errno is set > >> appropriately." So it should be returning an error, and logging a message for > >> a file it cannot unlink or people will be surprised with weird failures. > > > > Really need one message for unlink failed, but I'm afraid that if it > > make sense for return an error code when application exit. > > > > Thanks > > Michael > >> It also had some minor typos / English in the comments but we can fix that too. > >> > >> Matthew. > >> > > > > > > > Agree.May be it is not need to return error? > -- > Regards, > Haifeng > May I throw out a few extra ideas. The basic problem with DPDK and hugepages on exit, as you have already diagnosed, is not that the hugepages aren't released for re-use, its that the files inside the hugepage directory aren't unlinked. There are two considerations here that prevented us from coming up previously with a solution to this that we were fully happy with. 1. There is no automatic way (at least none that I'm aware of), to have the kernel automatically delete a file on exit. This means that any scheme to delete the files on application termination will have to be a voluntary one that won't happen if an app crashed or is forcibly terminated. That is why the EAL right now does the clean-up on the restart of the app instead. 2. The other consideration is multi-process. In a multi-process environment, it is unclear when exactly an app is finished - just because one process terminates does not mean that the whole app is finished with the hugepage files. If the files are deleted before a DPDK secondary process starts up, it won't be able to start as it won't be able to mmap the hugepage memory it needs. Now, that being said, I think we could see about having automatic hugepage deletion controlled by an EAL parameter. That way, the parameter could be omitted in the multi-process case, but could be used for those who want to have a clean hugepage dir after app termination. What I think we could do is, instead of having the app save the list of hugepages it uses as the app initializes, is to have the app delete the hugepage files as soon as it closes the filehandle for them. If my understanding is correct, the kernel will actually keep the hugepage memory around in the app as usual from that point onwards, and will only release it back [automatically] on app termination. New processes won't be able to mmap the file, but the running process will be unaffected. The best thing about this approach is that the hugepage file entries will be deleted whether or not the application terminates normally or crashes. So, any thoughts? Would such a scheme work? I haven't prototyped it, yet, but I think it should work. Regards, /Bruce PS: As well as multi-process not being able to use this scheme, I'd also note that this would prevent the dump_cfg utility app - or any other DPDK analysis app like it - from being used to inspect the memory area of the running process. That is another reason why I think the flag should not be set by default.