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 F0D8CA04AF for ; Thu, 17 Sep 2020 13:33:45 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DC2081D605; Thu, 17 Sep 2020 13:33:45 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id D96D41D605; Thu, 17 Sep 2020 13:33:43 +0200 (CEST) IronPort-SDR: JvECk8hU1ufRYDba4TMg/7xLsjYyDRLHOhNahFak6wQ3ecCsq9lmW4oKtmW12ygkroQL1eBLjn bYSco1iDWgtA== X-IronPort-AV: E=McAfee;i="6000,8403,9746"; a="244512696" X-IronPort-AV: E=Sophos;i="5.76,436,1592895600"; d="scan'208";a="244512696" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Sep 2020 04:33:41 -0700 IronPort-SDR: jDuxNs8B/SYVifiyTLCgmEwh6fIoZxaLQSLCwnrjscgAdz5JXdYS6IRB41IfIlUAsphOcWDkyk 8mYNutVgzp8g== X-IronPort-AV: E=Sophos;i="5.76,436,1592895600"; d="scan'208";a="483712211" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.213.206.43]) ([10.213.206.43]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Sep 2020 04:33:40 -0700 To: wangyunjian , "dev@dpdk.org" , "david.marchand@redhat.com" Cc: "Lilijun (Jerry)" , xudingke , "stable@dpdk.org" References: <1595515713-24640-1-git-send-email-wangyunjian@huawei.com> <34EFBCA9F01B0748BEB6B629CE643AE60D0EAAD8@dggemm513-mbx.china.huawei.com> <34EFBCA9F01B0748BEB6B629CE643AE60D0F695F@dggemm513-mbx.china.huawei.com> <21d092c3-83ce-a3f4-2411-193a5f46a111@intel.com> <34EFBCA9F01B0748BEB6B629CE643AE60D110040@DGGEMM533-MBX.china.huawei.com> From: "Burakov, Anatoly" Message-ID: Date: Thu, 17 Sep 2020 12:33:38 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <34EFBCA9F01B0748BEB6B629CE643AE60D110040@DGGEMM533-MBX.china.huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v2] eal/linux: do not create user mem map repeatedly when it exists X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 05-Aug-20 1:58 PM, wangyunjian wrote: >> -----Original Message----- >> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com] >> Sent: Friday, July 31, 2020 7:55 PM >> To: wangyunjian ; dev@dpdk.org; >> david.marchand@redhat.com >> Cc: Lilijun (Jerry) ; xudingke >> ; stable@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v2] eal/linux: do not create user mem map >> repeatedly when it exists >> >> On 30-Jul-20 2:16 PM, wangyunjian wrote: >>>> -----Original Message----- >>>> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com] >>>> Sent: Monday, July 27, 2020 5:24 PM >>>> To: wangyunjian ; dev@dpdk.org; >>>> david.marchand@redhat.com >>>> Cc: Lilijun (Jerry) ; xudingke >>>> ; stable@dpdk.org >>>> Subject: Re: [dpdk-dev] [PATCH v2] eal/linux: do not create user mem map >>>> repeatedly when it exists >>>> >>>> On 25-Jul-20 10:59 AM, wangyunjian wrote: >>>>>> -----Original Message----- >>>>>> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com] >>>>>> Sent: Friday, July 24, 2020 9:25 PM >>>>>> To: wangyunjian ; dev@dpdk.org; >>>>>> david.marchand@redhat.com >>>>>> Cc: Lilijun (Jerry) ; xudingke >>>>>> ; stable@dpdk.org >>>>>> Subject: Re: [dpdk-dev] [PATCH v2] eal/linux: do not create user mem >>>>>> map repeatedly when it exists >>>>>> >>>>>> On 23-Jul-20 3:48 PM, wangyunjian wrote: >>>>>>> From: Yunjian Wang >>>>>>> >>>>>>> Currently, we will create new user mem map entry for the same memory >>>>>>> segment, but in fact it has already been added to the user mem maps. >>>>>>> It's not necessary to create it twice. >>>>>>> >>>>>>> To resolve the issue, add support to remove the same entry in the >>>>>>> function compact_user_maps(). >>>>>>> >>>>>>> Fixes: 0cbce3a167f1 ("vfio: skip DMA map failure if already mapped") >>>>>>> Cc: stable@dpdk.org >>>>>>> >>>>>>> Signed-off-by: Yunjian Wang >>>>>>> --- >>>>>>> v2: >>>>>>> * Remove the same entry in the function compact_user_maps() >>>>>>> --- >>>>>>> lib/librte_eal/linux/eal_vfio.c | 5 +++++ >>>>>>> 1 file changed, 5 insertions(+) >>>>>>> >>>>>>> diff --git a/lib/librte_eal/linux/eal_vfio.c >>>>>>> b/lib/librte_eal/linux/eal_vfio.c index abb12a354..df99307b7 100644 >>>>>>> --- a/lib/librte_eal/linux/eal_vfio.c >>>>>>> +++ b/lib/librte_eal/linux/eal_vfio.c >>>>>>> @@ -167,6 +167,10 @@ adjust_map(struct user_mem_map *src, >> struct >>>>>> user_mem_map *end, >>>>>>> static int >>>>>>> merge_map(struct user_mem_map *left, struct user_mem_map >>>> *right) >>>>>>> { >>>>>>> + /* merge the same maps into one */ >>>>>>> + if (memcmp(left, right, sizeof(struct user_mem_map)) == 0) >>>>>>> + goto out; >>>>>>> + >>>>>> >>>>>> merge_map looks for adjacent maps only, but does not handle maps that >>>>>> are wholly contained within one another ("the same map" also matches >>>>>> this definition). wouldn't it be better to check for that instead of >>>>>> *just* handling identical maps? >>>>> >>>>> What about using the initial implementation? >>>>> We don't create new user mem map entry for the same memory segment. >>>> >>>> I don't like this implementation because it relies on particulars of how VFIO >>>> mapping work without explicitly specifying them. I.e. it's prone to breaking >>>> when changing code. That's not even mentioning that we have no >> guarantees >>>> on kernel behavior in that particular case being identical on all supported >>>> platforms. >>>> >>>> I would honestly prefer an explicit compaction over implicit one. >>> >>> What about this implementation? >> >> Again, this works, but i feel like specializing it to just merge the >> exact same maps is missing an opportunity to provide a more general >> solution that merges same *and* subset maps. > > Currently, the problem that I encounter is that a container has many > devices and the application will map the same memory many times. > The kernel driver returns EEXIST as long as there are overlapping memory > areas. Therefore, the application needs to ensure that the memory blocks > of the DMA do not overlap. Otherwise, it will not work normally. > > Could you offer me some ideas or advise to fix it？ > It sounds like your approach is better if that is indeed the case. -- Thanks, Anatoly