| Message ID | CADNuJVo6=MuC5pJV4ew8Rr1Mu=PwUUsB=QwgKaRDcCZO6VBfhg@mail.gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id F25B16A95; Thu, 11 Dec 2014 17:05:53 +0100 (CET) Received: from mail-yk0-f172.google.com (mail-yk0-f172.google.com [209.85.160.172]) by dpdk.org (Postfix) with ESMTP id E2E8868BE for <dev@dpdk.org>; Thu, 11 Dec 2014 17:05:51 +0100 (CET) Received: by mail-yk0-f172.google.com with SMTP id 131so2317532ykp.3 for <dev@dpdk.org>; Thu, 11 Dec 2014 08:05:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:date:message-id:subject:from:to :content-type; bh=HztzdfEBIXQeVv7ugQqaBJjr9lumtRFK6Zwmlge+DNI=; b=LK9PuBjOYxxaHtQ3wwQFE0zBEMP+vgAdQ9O5uIOCf17oma/1g/9YhXfkacqxRjHIfT StweGTyJHKZ9QT/CJYFS8/Sf+pOTy/dxaXscVeP2AycP3lfOLC6mFrEDgrzdyJiZz8Y3 b4VpuQToMpgGPXYqekPcmQVdBIRTgtTjoKwIbfb1O/dLZ5vzUI3AEFack8PQ//fNbXPF u6xW5XyOdEYpzf4N7grrLx5hia9CS4uLvKeTPD8CCugz+gTInOZDLRolyQECKoRLaaOT XBPEHjOMHGY6VhY0VPS7SnI4rB1CBz+ZE9B5PgDVu8cegGEdcBwp4UgGAT+noTHlpK4c XcIw== X-Gm-Message-State: ALoCoQmXLbbOGs0/ZclsUh9nX4isXpHOd3L9mRNOSxVQnc1lTuESkGGl/vvlyJdjBfm4arp7jo7H MIME-Version: 1.0 X-Received: by 10.236.32.40 with SMTP id n28mr7514877yha.16.1418313951303; Thu, 11 Dec 2014 08:05:51 -0800 (PST) Received: by 10.170.54.78 with HTTP; Thu, 11 Dec 2014 08:05:51 -0800 (PST) Date: Thu, 11 Dec 2014 10:05:51 -0600 Message-ID: <CADNuJVo6=MuC5pJV4ew8Rr1Mu=PwUUsB=QwgKaRDcCZO6VBfhg@mail.gmail.com> From: Jay Rolette <rolette@infiniteio.com> To: Dev <dev@dpdk.org> Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: [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 <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Jay Rolette
Dec. 11, 2014, 4:05 p.m. UTC
Signed-off-by: Jay Rolette <rolette@infiniteio.com>
---
lib/librte_eal/linuxapp/eal/eal_memory.c | 59
+++++++++++---------------------
1 file changed, 20 insertions(+), 39 deletions(-)
}
--
Comments
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette > Sent: Thursday, December 11, 2014 5:06 PM > To: Dev > Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with > qsort() from standard library > > Signed-off-by: Jay Rolette <rolette@infiniteio.com> > --- > 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; > +} > + Why not simply return (int)(p1->physaddr - p2->physaddr);
Because it doesn't work correctly :-) On Mon, Dec 15, 2014 at 3:05 AM, Wodkowski, PawelX < pawelx.wodkowski@intel.com> wrote: > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette > > Sent: Thursday, December 11, 2014 5:06 PM > > To: Dev > > Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() > with > > qsort() from standard library > > > > Signed-off-by: Jay Rolette <rolette@infiniteio.com> > > --- > > 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; > > +} > > + > > Why not simply > > return (int)(p1->physaddr - p2->physaddr); > >
> Because it doesn't work correctly :-)
It should, what I am missing here? :P
Pawel
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wodkowski, PawelX > Sent: Monday, December 15, 2014 9:05 AM > To: Jay Rolette; Dev > Subject: Re: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with qsort() from standard library > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette > > Sent: Thursday, December 11, 2014 5:06 PM > > To: Dev > > Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with > > qsort() from standard library > > > > Signed-off-by: Jay Rolette <rolette@infiniteio.com> > > --- > > 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; > > +} > > + > > Why not simply > > return (int)(p1->physaddr - p2->physaddr); physaddr is uint64_t.
FWIW, it surprised the heck out of me as well. Turns out that even though I'm compiling in 64-bit mode, gcc has sizeof(int) as 4 bytes rather than 8. Not really sure what the scoop is on that, but the standard leaves that up to the compiler. I'm used to int always being the "natural size" on the CPU/OS, so for a 64-bit executable on Intel, I assumed int would be 64-bit. Being an embedded developer for so many years, I rarely use semi-defined data types just to avoid these sorts of problems. On Mon, Dec 15, 2014 at 7:20 AM, Wodkowski, PawelX < pawelx.wodkowski@intel.com> wrote: > > > Because it doesn't work correctly :-) > > It should, what I am missing here? :P > > Pawel > >
Interestingly, in 64-bit mode the default size of operands on IA is still 32-bit. Instructions often need to have the REX prefix on them to actually use 64-bit data. The REX prefix is explained in section 2.2.1 of the "Intel® 64 and IA-32 Architectures Software Developer’s Manual", Volume 2 Regards, /Bruce -----Original Message----- From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette Sent: Monday, December 15, 2014 2:29 PM To: Wodkowski, PawelX Cc: Dev Subject: Re: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with qsort() from standard library FWIW, it surprised the heck out of me as well. Turns out that even though I'm compiling in 64-bit mode, gcc has sizeof(int) as 4 bytes rather than 8. Not really sure what the scoop is on that, but the standard leaves that up to the compiler. I'm used to int always being the "natural size" on the CPU/OS, so for a 64-bit executable on Intel, I assumed int would be 64-bit. Being an embedded developer for so many years, I rarely use semi-defined data types just to avoid these sorts of problems. On Mon, Dec 15, 2014 at 7:20 AM, Wodkowski, PawelX < pawelx.wodkowski@intel.com> wrote: > > > Because it doesn't work correctly :-) > > It should, what I am missing here? :P > > Pawel > >
Hi Jay, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette > Sent: Thursday, December 11, 2014 4:06 PM > To: Dev > Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with qsort() from standard library > > Signed-off-by: Jay Rolette <rolette@infiniteio.com> The patch itself looks good to me. Though it seems something wrong with formatting - all lines start with offset 0. Probably your mail client? Konstantin > --- > 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; > +} > + > /* > * 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; > } > > --
Thanks Konstantin. Yes, I'll resend. Not sure why gmail is removing whitespace when I sent in Plain Text mode. Ultimately I'll need to figure out how to properly configure git to send these directly instead of handling them more manually. The examples I saw assumed you were using a gmail.com email rather than a corporate email hosted via google apps. Jay On Tue, Dec 16, 2014 at 12:39 PM, Ananyev, Konstantin < konstantin.ananyev@intel.com> wrote: > > > Hi Jay, > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette > > Sent: Thursday, December 11, 2014 4:06 PM > > To: Dev > > Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() > with qsort() from standard library > > > > Signed-off-by: Jay Rolette <rolette@infiniteio.com> > > The patch itself looks good to me. > Though it seems something wrong with formatting - all lines start with > offset 0. > Probably your mail client? > Konstantin > > > > --- > > 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; > > +} > > + > > /* > > * 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; > > } > > > > -- >
Actually, I just relooked at the email I sent and it looks correct (properly indented, etc.). Any suggestions for what might be going on? On Tue, Dec 16, 2014 at 1:18 PM, Jay Rolette <rolette@infiniteio.com> wrote: > > Thanks Konstantin. Yes, I'll resend. Not sure why gmail is removing > whitespace when I sent in Plain Text mode. > > Ultimately I'll need to figure out how to properly configure git to send > these directly instead of handling them more manually. The examples I saw > assumed you were using a gmail.com email rather than a corporate email > hosted via google apps. > > Jay > > On Tue, Dec 16, 2014 at 12:39 PM, Ananyev, Konstantin < > konstantin.ananyev@intel.com> wrote: >> >> >> Hi Jay, >> >> > -----Original Message----- >> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette >> > Sent: Thursday, December 11, 2014 4:06 PM >> > To: Dev >> > Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() >> with qsort() from standard library >> > >> > Signed-off-by: Jay Rolette <rolette@infiniteio.com> >> >> The patch itself looks good to me. >> Though it seems something wrong with formatting - all lines start with >> offset 0. >> Probably your mail client? >> Konstantin >> >> >> > --- >> > 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; >> > +} >> > + >> > /* >> > * 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; >> > } >> > >> > -- >> >
Hi Jay, From: Jay Rolette [mailto:rolette@infiniteio.com] Sent: Tuesday, December 16, 2014 7:21 PM To: Ananyev, Konstantin Cc: Dev Subject: Re: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with qsort() from standard library Actually, I just relooked at the email I sent and it looks correct (properly indented, etc.). Any suggestions for what might be going on? On Tue, Dec 16, 2014 at 1:18 PM, Jay Rolette <rolette@infiniteio.com<mailto:rolette@infiniteio.com>> wrote: Thanks Konstantin. Yes, I'll resend. Not sure why gmail is removing whitespace when I sent in Plain Text mode. Sorry, don’t know either. Wonder, did you see this: https://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html There is a section about different mailers, etc. Konstantin Ultimately I'll need to figure out how to properly configure git to send these directly instead of handling them more manually. The examples I saw assumed you were using a gmail.com<http://gmail.com> email rather than a corporate email hosted via google apps. Jay On Tue, Dec 16, 2014 at 12:39 PM, Ananyev, Konstantin <konstantin.ananyev@intel.com<mailto:konstantin.ananyev@intel.com>> wrote: Hi Jay, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>] On Behalf Of Jay Rolette > Sent: Thursday, December 11, 2014 4:06 PM > To: Dev > Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with qsort() from standard library > > Signed-off-by: Jay Rolette <rolette@infiniteio.com<mailto:rolette@infiniteio.com>> The patch itself looks good to me. Though it seems something wrong with formatting - all lines start with offset 0. Probably your mail client? Konstantin > --- > 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; > +} > + > /* > * 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; > } > > --
On 12/17/2014 7:01 PM, Ananyev, Konstantin wrote: > Hi Jay, > > From: Jay Rolette [mailto:rolette@infiniteio.com] > Sent: Tuesday, December 16, 2014 7:21 PM > To: Ananyev, Konstantin > Cc: Dev > Subject: Re: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with qsort() from standard library > > Actually, I just relooked at the email I sent and it looks correct (properly indented, etc.). Any suggestions for what might be going on? > > On Tue, Dec 16, 2014 at 1:18 PM, Jay Rolette <rolette@infiniteio.com<mailto:rolette@infiniteio.com>> wrote: > Thanks Konstantin. Yes, I'll resend. Not sure why gmail is removing whitespace when I sent in Plain Text mode. > > Sorry, don’t know either. > Wonder, did you see this: > https://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html > There is a section about different mailers, etc. > Konstantin > > Ultimately I'll need to figure out how to properly configure git to send these directly instead of handling them more manually. The examples I saw assumed you were using a gmail.com<http://gmail.com> email rather than a corporate email hosted via google apps. Hi Jay, I was ever use git send-email with my gmail account, it works. So do you config your git send-email correctly? Thanks, Michael > Jay > > On Tue, Dec 16, 2014 at 12:39 PM, Ananyev, Konstantin <konstantin.ananyev@intel.com<mailto:konstantin.ananyev@intel.com>> wrote: > > Hi Jay, > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>] On Behalf Of Jay Rolette >> Sent: Thursday, December 11, 2014 4:06 PM >> To: Dev >> Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() with qsort() from standard library >> >> Signed-off-by: Jay Rolette <rolette@infiniteio.com<mailto:rolette@infiniteio.com>> > The patch itself looks good to me. > Though it seems something wrong with formatting - all lines start with offset 0. > Probably your mail client? > Konstantin > > >> --- >> 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; >> +} >> + >> /* >> * 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; >> } >> >> --
Thanks to some help from Matthew Hall, it looks like I have it working now. I just resent the patch directly from git. Please let me know if it looks ok now? Sorry for the hassles. We use Mercurial internally, so while there is a lot of overlap, sending patches isn't something I have to worry about day-to-day. Appreciate the help getting things configured! Jay On Wed, Dec 17, 2014 at 7:08 AM, Qiu, Michael <michael.qiu@intel.com> wrote: > > On 12/17/2014 7:01 PM, Ananyev, Konstantin wrote: > > Hi Jay, > > > > From: Jay Rolette [mailto:rolette@infiniteio.com] > > Sent: Tuesday, December 16, 2014 7:21 PM > > To: Ananyev, Konstantin > > Cc: Dev > > Subject: Re: [dpdk-dev] [PATCH] replaced O(n^2) sort in > sort_by_physaddr() with qsort() from standard library > > > > Actually, I just relooked at the email I sent and it looks correct > (properly indented, etc.). Any suggestions for what might be going on? > > > > On Tue, Dec 16, 2014 at 1:18 PM, Jay Rolette <rolette@infiniteio.com > <mailto:rolette@infiniteio.com>> wrote: > > Thanks Konstantin. Yes, I'll resend. Not sure why gmail is removing > whitespace when I sent in Plain Text mode. > > > > Sorry, don’t know either. > > Wonder, did you see this: > > https://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html > > There is a section about different mailers, etc. > > Konstantin > > > > Ultimately I'll need to figure out how to properly configure git to send > these directly instead of handling them more manually. The examples I saw > assumed you were using a gmail.com<http://gmail.com> email rather than a > corporate email hosted via google apps. > Hi Jay, > > I was ever use git send-email with my gmail account, it works. > > So do you config your git send-email correctly? > > Thanks, > Michael > > > > Jay > > > > On Tue, Dec 16, 2014 at 12:39 PM, Ananyev, Konstantin < > konstantin.ananyev@intel.com<mailto:konstantin.ananyev@intel.com>> wrote: > > > > Hi Jay, > > > >> -----Original Message----- > >> From: dev [mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>] > On Behalf Of Jay Rolette > >> Sent: Thursday, December 11, 2014 4:06 PM > >> To: Dev > >> Subject: [dpdk-dev] [PATCH] replaced O(n^2) sort in sort_by_physaddr() > with qsort() from standard library > >> > >> Signed-off-by: Jay Rolette <rolette@infiniteio.com<mailto: > rolette@infiniteio.com>> > > The patch itself looks good to me. > > Though it seems something wrong with formatting - all lines start with > offset 0. > > Probably your mail client? > > Konstantin > > > > > >> --- > >> 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; > >> +} > >> + > >> /* > >> * 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; > >> } > >> > >> -- > >
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; +} + /* * 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;