From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yk0-f182.google.com (mail-yk0-f182.google.com [209.85.160.182]) by dpdk.org (Postfix) with ESMTP id 15E9B68AA for ; Wed, 17 Dec 2014 16:07:46 +0100 (CET) Received: by mail-yk0-f182.google.com with SMTP id 131so6916987ykp.41 for ; Wed, 17 Dec 2014 07:07:45 -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:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=j3fa8Lmcutes6RTggYDKNPZcEA3yxCgTWvQPSZ/lG6Q=; b=JZv44B2Ffvq6Oo1BlSQjMN1uKRfhf7zO9ROH3xdmcUtMefINtm9SeDyOfBHyHWWPo5 MtQclpDaM3Vu7NlE0rpUelZ3vFoHuWNt0BU+9CQ+rM/bmI8WrbfSq9XJwiSiC773uxOa 5ZNpJLpOpKuiuSUiun15SYXdOD4KDnwGas4V65Izx8wntRB90AtivZtyJt3ZdxWfc8Xj huANIC8iKTo2xWG7k0oB5LO5Y1teIUvVIQaWLufexICc6heYMX6EzMdXKGbJeOA/ZuAK YqXbusP07+SMdxyDeAWymn/C4vCkpr7EyJzChdrO9dSLOgLh+Pm7ky5imnkyoqpszR4p gMgg== X-Gm-Message-State: ALoCoQnUcE6rbJCacOEGerM49H0M2+Se941+oUbCey4AVq1lXuj1li1DLZt0vS5YF25dlD1ZrQv3 MIME-Version: 1.0 X-Received: by 10.170.63.212 with SMTP id f203mr28095966ykf.63.1418828865471; Wed, 17 Dec 2014 07:07:45 -0800 (PST) Received: by 10.170.54.78 with HTTP; Wed, 17 Dec 2014 07:07:45 -0800 (PST) In-Reply-To: <1667884.VTnHUhe7ya@xps13> References: <1418823077-9129-1-git-send-email-rolette@infiniteio.com> <1667884.VTnHUhe7ya@xps13> Date: Wed, 17 Dec 2014 09:07:45 -0600 Message-ID: From: Jay Rolette To: Thomas Monjalon Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: Dev 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, 17 Dec 2014 15:07:46 -0000 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