From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 8A204C428 for ; Wed, 21 Oct 2015 18:34:26 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP; 21 Oct 2015 09:34:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,712,1437462000"; d="scan'208";a="668910230" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.208.65]) by orsmga003.jf.intel.com with SMTP; 21 Oct 2015 09:34:24 -0700 Received: by (sSMTP sendmail emulation); Wed, 21 Oct 2015 17:34:22 +0025 Date: Wed, 21 Oct 2015 17:34:22 +0100 From: Bruce Richardson To: "shesha Sreenivasamurthy (shesha)" Message-ID: <20151021163422.GA10344@bricha3-MOBL3> References: <1445419101-19690-1-git-send-email-shesha@cisco.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v3] mem: command line option to delete hugepage backing files 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, 21 Oct 2015 16:34:27 -0000 On Wed, Oct 21, 2015 at 04:22:45PM +0000, shesha Sreenivasamurthy (shesha) wrote: > When an application using huge-pages crash or exists, the hugetlbfs > backing files are not cleaned up. This is a patch to clean those files. > There are multi-process DPDK applications that may be benefited by those > backing files. Therefore, I have made that configurable so that the > application that does not need those backing files can remove them, thus > not changing the current default behavior. The application itself can > clean it up, however the rationale behind DPDK cleaning it up is, DPDK > created it and therefore, it is better it unlinks it. > > Signed-off-by: Shesha Sreenivasamurthy > --- > lib/librte_eal/common/eal_common_options.c | 12 ++++++++++++ > lib/librte_eal/common/eal_internal_cfg.h | 1 + > lib/librte_eal/common/eal_options.h | 2 ++ > lib/librte_eal/linuxapp/eal/eal_memory.c | 30 > ++++++++++++++++++++++++++++++ > 4 files changed, 45 insertions(+) > > +static int > +unlink_hugepage_files(struct hugepage_file *hugepg_tbl, > + unsigned num_hp_info) > +{ > + unsigned socket, size; > + int page, nrpages = 0; > + > + /* get total number of hugepages */ > + for (size = 0; size < num_hp_info; size++) > + for (socket = 0; socket < RTE_MAX_NUMA_NODES; socket++) > + nrpages += internal_config.hugepage_info[size].num_pages[socket]; > + > + for (page = 0; page < nrpages; page++) { > + struct hugepage_file *hp = &hugepg_tbl[page]; > + if (hp->final_va != NULL && unlink(hp->filepath)) { > + RTE_LOG(WARNING, EAL, "%s(): Removing %s failed: %s\n", > + __func__, hp->filepath, strerror(errno)); > + } > + } > + return 0; > +} > + > /* > * unmaps hugepages that are not going to be used. since we originally > allocate > * ALL hugepages (not just those we need), additional unmapping needs to > be done. > @@ -1289,6 +1311,14 @@ rte_eal_hugepage_init(void) > goto fail; > } > > + /* free the hugepage backing files */ > + if (internal_config.hugepage_unlink && > + unlink_hugepage_files(tmp_hp, > + internal_config.num_hugepage_sizes) < 0) { > + RTE_LOG(ERR, EAL, "Unlinking hugepage backing files failed!\n"); > + goto fail; > + } > + Sorry for the late comment, but... Rather than adding a whole new function to be called here, can the same effect not be got by adding in 2/3 lines like: if (internal_config.hugepage_unlink) unlink(hugetlb[i].filepath) at line 409 of eal_memory.c where were have done our final mmap of the file. [You also need the same couple of lines for the 32-bit special case at line 351]. It would be a shorter diff. /Bruce