From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jay Rolette <rolette-bIuJOMs36aleGPcbtGPokg@public.gmane.org>
Subject: Re: [PATCH] replaced O(n^2) sort in sort_by_physaddr()
 with qsort() from standard library
Date: Wed, 17 Dec 2014 09:07:45 -0600
Message-ID: <CADNuJVpObspy7zW6D4Aww8LZNezgP8twrYp_Hej5ZFD1+L0ZKQ@mail.gmail.com>
References: <1418823077-9129-1-git-send-email-rolette@infiniteio.com>
 <1667884.VTnHUhe7ya@xps13>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Cc: Dev <dev-VfR2kkLFssw@public.gmane.org>
To: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <1667884.VTnHUhe7ya@xps13>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

Hi Thomas,

Please read http://dpdk.org/dev#send for submission guidelines.
>

I did when I was figuring out how to submit the patch, but possible I'm
missing something on the tools to get it to include the commit comment
correctly.

A description of why you do it would be welcome in the commit log.
>

How much are you looking for here? I thought replacing an O(n^2) algorithm
with qsort() was fairly self-evident. Less code and take advantage of
standard library code that is faster.

I get it in the general case. Just didn't seem necessary for this one.


> > +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
>
> Comments shall be C-style (/* */).
>

Single line comments ('//') have been part of the C standard since C99. Is
DPDK following C89 or is this just a style thing? If it is a style thing, a
link to a page with the rubric would be helpful. I didn't see one on the
submission guidelines.


> > +     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;
> > +}
>
> One of the goal of EAL is to avoid #ifdef.
> So that function would probably be better located in
> lib/librte_eal/common/include/arch/* with different implemenations
> depending of the architecture.
>

Hmm... I was following the approach already used in the module. See
map_all_hugepages(), remap_all_hugepages().

Regards,
Jay