From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 3712A19F5 for ; Mon, 17 Sep 2018 11:55:19 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Sep 2018 02:55:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,384,1531810800"; d="scan'208";a="84150557" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.55]) ([10.237.220.55]) by orsmga003.jf.intel.com with ESMTP; 17 Sep 2018 02:53:57 -0700 To: Maxime Coquelin , dev@dpdk.org Cc: Bruce Richardson , tiwei.bie@intel.com, ray.kinsella@intel.com, zhihong.wang@intel.com, kuralamudhan.ramakrishnan@intel.com References: <6a24b2f23baccc78b7bab7ecfee84d3ed477ab80.1536073997.git.anatoly.burakov@intel.com> <52ef741c-3b94-295b-9ec0-dafe4cb63b3c@redhat.com> From: "Burakov, Anatoly" Message-ID: Date: Mon, 17 Sep 2018 10:53:56 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <52ef741c-3b94-295b-9ec0-dafe4cb63b3c@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 6/9] memalloc: add EAL-internal API to get and set segment fd's X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Sep 2018 09:55:19 -0000 On 14-Sep-18 8:54 AM, Maxime Coquelin wrote: > > > On 09/04/2018 05:15 PM, Anatoly Burakov wrote: >> Enable setting and retrieving segment fd's internally. >> >> For now, retrieving fd's will not be used anywhere until we >> get an external API, but it will be useful for things like >> virtio, where we wish to share segment fd's. >> >> Setting segment fd's will not be available as a public API >> at this time, but internally it is needed for legacy mode, >> because we're not allocating our hugepages in memalloc in >> legacy mode case, and we still need to store the fd. >> >> Another user of get segment fd API is memseg info dump, to >> show which pages use which fd's. >> >> Not supported on FreeBSD. >> >> Signed-off-by: Anatoly Burakov >> --- >>   lib/librte_eal/bsdapp/eal/eal_memalloc.c   | 12 +++++ >>   lib/librte_eal/common/eal_common_memory.c  |  8 +-- >>   lib/librte_eal/common/eal_memalloc.h       |  6 +++ >>   lib/librte_eal/linuxapp/eal/eal_memalloc.c | 60 +++++++++++++++++----- >>   lib/librte_eal/linuxapp/eal/eal_memory.c   | 44 +++++++++++++--- >>   5 files changed, 109 insertions(+), 21 deletions(-) >> >> diff --git a/lib/librte_eal/bsdapp/eal/eal_memalloc.c >> b/lib/librte_eal/bsdapp/eal/eal_memalloc.c >> index f7f07abd6..a5fb09f71 100644 >> --- a/lib/librte_eal/bsdapp/eal/eal_memalloc.c >> +++ b/lib/librte_eal/bsdapp/eal/eal_memalloc.c >> @@ -47,6 +47,18 @@ eal_memalloc_sync_with_primary(void) >>       return -1; >>   } >> +int >> +eal_memalloc_get_seg_fd(int list_idx, int seg_idx) >> +{ >> +    return -1; > > Why not returning -ENOTSUPP directly here? (see patch 7). > >> +} >> + >> +int >> +eal_memalloc_set_seg_fd(int list_idx, int seg_idx, int fd) >> +{ >> +    return -1; > > Ditto. > >> +} >> + > > Other than that, the patch looks good to me. > > Maxime > Mainly for consistency reasons. Returning errno values but not using them looks weird to me, but i can change this to return those at the outset. -- Thanks, Anatoly