From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 839003B5 for ; Wed, 9 Sep 2015 12:36:15 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 09 Sep 2015 03:35:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,496,1437462000"; d="scan'208";a="785719953" Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.221.35]) ([10.237.221.35]) by fmsmga001.fm.intel.com with ESMTP; 09 Sep 2015 03:35:58 -0700 From: "Gonzalez Monroy, Sergio" To: Jay Rolette References: <1418823077-9129-1-git-send-email-rolette@infiniteio.com> Message-ID: <55F00B8D.9010704@intel.com> Date: Wed, 9 Sep 2015 11:35:57 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <1418823077-9129-1-git-send-email-rolette@infiniteio.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with qsort() from standard library 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, 09 Sep 2015 10:36:16 -0000 Following conversation in http://dpdk.org/ml/archives/dev/2015-September/023230.html : On 17/12/2014 13:31, rolette at infiniteio.com (Jay Rolette) wrote: > Signed-off-by: Jay Rolette > --- Update commit title/description, maybe something like: eal/linux: use qsort for sorting hugepages Replace O(n^2) sort in sort_by_physaddr() with qsort() from standard library > lib/librte_eal/linuxapp/eal/eal_memory.c | 59 +++++++++++--------------------- > 1 file changed, 20 insertions(+), 39 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c > index bae2507..3656515 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_memory.c > +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c > @@ -670,6 +670,25 @@ error: > return -1; > } > > +static int > +cmp_physaddr(const void *a, const void *b) > +{ > +#ifndef RTE_ARCH_PPC_64 > + const struct hugepage_file *p1 = (const struct hugepage_file *)a; > + const struct hugepage_file *p2 = (const struct hugepage_file *)b; > +#else > + // PowerPC needs memory sorted in reverse order from x86 > + const struct hugepage_file *p1 = (const struct hugepage_file *)b; > + const struct hugepage_file *p2 = (const struct hugepage_file *)a; > +#endif > + if (p1->physaddr < p2->physaddr) > + return -1; > + else if (p1->physaddr > p2->physaddr) > + return 1; > + else > + return 0; > +} > + There were a couple of comments from Thomas about the comments style and #ifdef: - Comment style should be modified as per http://dpdk.org/doc/guides/contributing/coding_style.html#c-comment-style - Regarding the #ifdef, I agree with Jay that it is the current approach in the file. > /* > * Sort the hugepg_tbl by physical address (lower addresses first on x86, > * higher address first on powerpc). We use a slow algorithm, but we won't > @@ -678,45 +697,7 @@ error: > static int > sort_by_physaddr(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi) > { > - unsigned i, j; > - int compare_idx; > - uint64_t compare_addr; > - struct hugepage_file tmp; > - > - for (i = 0; i < hpi->num_pages[0]; i++) { > - compare_addr = 0; > - compare_idx = -1; > - > - /* > - * browse all entries starting at 'i', and find the > - * entry with the smallest addr > - */ > - for (j=i; j< hpi->num_pages[0]; j++) { > - > - if (compare_addr == 0 || > -#ifdef RTE_ARCH_PPC_64 > - hugepg_tbl[j].physaddr > compare_addr) { > -#else > - hugepg_tbl[j].physaddr < compare_addr) { > -#endif > - compare_addr = hugepg_tbl[j].physaddr; > - compare_idx = j; > - } > - } > - > - /* should not happen */ > - if (compare_idx == -1) { > - RTE_LOG(ERR, EAL, "%s(): error in physaddr sorting\n", __func__); > - return -1; > - } > - > - /* swap the 2 entries in the table */ > - memcpy(&tmp, &hugepg_tbl[compare_idx], > - sizeof(struct hugepage_file)); > - memcpy(&hugepg_tbl[compare_idx], &hugepg_tbl[i], > - sizeof(struct hugepage_file)); > - memcpy(&hugepg_tbl[i], &tmp, sizeof(struct hugepage_file)); > - } > + qsort(hugepg_tbl, hpi->num_pages[0], sizeof(struct hugepage_file), cmp_physaddr); > return 0; > } Why not just remove sort_by_physaddr() and call qsort() directly? Sergio