From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0E1EDA0539; Thu, 23 Jan 2020 14:06:21 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D9D0A343C; Thu, 23 Jan 2020 14:06:19 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id B83BF2BAE for ; Thu, 23 Jan 2020 14:06:17 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Jan 2020 05:06:16 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,354,1574150400"; d="scan'208";a="227993230" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.237.220.124]) ([10.237.220.124]) by orsmga003.jf.intel.com with ESMTP; 23 Jan 2020 05:06:15 -0800 To: Maxime Coquelin , dev@dpdk.org, tiwei.bie@intel.com, zhihong.wang@intel.com References: <20191213141322.32730-1-maxime.coquelin@redhat.com> <20191213141322.32730-2-maxime.coquelin@redhat.com> From: "Burakov, Anatoly" Message-ID: <0583a91b-1250-8df7-ece5-3b9693c5e587@intel.com> Date: Thu, 23 Jan 2020 13:06:14 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20191213141322.32730-2-maxime.coquelin@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/4] eal: add new API to register contiguous external memory 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 13-Dec-19 2:13 PM, Maxime Coquelin wrote: > This new API allows to pass a file descriptor while > registering external and contiguous memory in the IOVA space. > > This is required for using Virtio-user PMD with application > using external memory for the mbuf's buffers, like Seastar > or VPP. > > FD is only attached to the segments if single_file_segment > option is enabled. > > Signed-off-by: Maxime Coquelin Hi Maxime, We've discussed this privately already, but i'll send feedback anyway just so that it's visible. > --- > lib/librte_eal/common/eal_common_memory.c | 75 +++++++++++++++++++++- > lib/librte_eal/common/include/rte_memory.h | 46 +++++++++++++ > lib/librte_eal/common/malloc_heap.c | 17 ++++- > lib/librte_eal/common/malloc_heap.h | 2 +- > lib/librte_eal/common/rte_malloc.c | 2 +- > lib/librte_eal/rte_eal_version.map | 3 + > 6 files changed, 141 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c > index 4a9cc1f19a..7a4b371828 100644 > --- a/lib/librte_eal/common/eal_common_memory.c > +++ b/lib/librte_eal/common/eal_common_memory.c > @@ -772,6 +772,79 @@ rte_memseg_get_fd_offset(const struct rte_memseg *ms, size_t *offset) > return ret; > } > > +int > +rte_extmem_register_contig(void *va_addr, size_t len, rte_iova_t iova_addr, > + size_t page_sz, int fd) > +{ I don't see why do you have to link IOVA-contiguousness with setting an fd - a chunk of memory backed by an fd does not necessarily have to be IOVA-contiguous. In fact, registering IOVA-contiguous memory is already possible - you just specify the IOVA table such that each successive IOVA is contiguous to the preceding one. So, IMO, the 'fd' part should be decoupled from registering. Also, we support two modes of operation (fd per segment and fd per segment list), it would be nice to have both. For example, consider the following: // set fd for an entire segment list, addressed by VA and len. // must match a memory chunk that was registered earlier rte_extmem_set_seg_fd(..., int fd); // set per-segment fd's for the segment list, addressed by VA and len. // must match a memory chunk that was registered earlier, and length of // fd array must add up to length of the chunk rte_extmem_set_seg_fd_list(..., int *fd, int len); This would essentially accomplish what you're trying to do by just adding the missing piece required. Question: do we want to be able to set segment fd's for externally allocated by internally managed memory? E.g. something that was added via malloc_heap_add_memory rather than extmem_register? I would guess so, so maybe it shouldn't be under rte_extmem namespace. -- Thanks, Anatoly