> -----Original Message----- > From: Burakov, Anatoly [mailto:anatoly.bura...@intel.com] > Sent: Monday, July 20, 2020 7:46 PM > To: wangyunjian <wangyunj...@huawei.com>; dev@dpdk.org; > david.march...@redhat.com > Cc: Lilijun (Jerry) <jerry.lili...@huawei.com>; xudingke > <xudin...@huawei.com>; sta...@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/1] eal/linux: do not create user mem map > repeatedly when it exists > > On 20-Jul-20 3:00 AM, wangyunjian wrote: > >> -----Original Message----- > >> From: Burakov, Anatoly [mailto:anatoly.bura...@intel.com] > >> Sent: Friday, July 17, 2020 10:24 PM > >> To: wangyunjian <wangyunj...@huawei.com>; dev@dpdk.org; > >> david.march...@redhat.com > >> Cc: Lilijun (Jerry) <jerry.lili...@huawei.com>; xudingke > >> <xudin...@huawei.com>; sta...@dpdk.org > >> Subject: Re: [dpdk-dev] [PATCH 1/1] eal/linux: do not create user mem > >> map repeatedly when it exists > >> > >> On 17-Jul-20 3:19 PM, Burakov, Anatoly wrote: > >>> On 16-Jul-20 2:38 PM, wangyunjian wrote: > >>>> From: Yunjian Wang <wangyunj...@huawei.com> > >>>> > >>>> 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. > >>>> > >>>> Fixes: 0cbce3a167f1 ("vfio: skip DMA map failure if already > >>>> mapped") > >>>> Cc: sta...@dpdk.org > >>>> > >>>> Signed-off-by: Yunjian Wang <wangyunj...@huawei.com> > >>>> --- > >>>> lib/librte_eal/linux/eal_vfio.c | 7 +++++++ > >>>> 1 file changed, 7 insertions(+) > >>>> > >>>> diff --git a/lib/librte_eal/linux/eal_vfio.c > >>>> b/lib/librte_eal/linux/eal_vfio.c index abb12a354..d8a8c39ab 100644 > >>>> --- a/lib/librte_eal/linux/eal_vfio.c > >>>> +++ b/lib/librte_eal/linux/eal_vfio.c > >>>> @@ -1828,6 +1828,13 @@ container_dma_map(struct vfio_config > >>>> *vfio_cfg, uint64_t vaddr, uint64_t iova, > >>>> ret = -1; > >>>> goto out; > >>>> } > >>>> + > >>>> + /* we don't need create new user mem map entry > >>>> + * for the same memory segment. > >>>> + */ > >>>> + if (errno == EBUSY || errno == EEXIST) > >>>> + goto out; > >>>> + > >>> > >>> I'm not sure i understand this patch. If we get errno, the call has > >>> failed, which means we're doing "goto out" from a few lines above. > >>> Am i missing something here? > >>> > >>>> /* create new user mem map entry */ > >>>> new_map = > >> &user_mem_maps->maps[user_mem_maps->n_maps++]; > >>>> new_map->addr = vaddr; > >>>> > >>> > >>> > >> > >> Oh, i see, the actual functions will set errno and return 0. > >> > >> I don't think it's an actual issue as compacting will presumably > >> remove the extra user mem map anyway. What exactly is being fixed > >> here? Does compacting user mem maps not remove the extra entry? > > > > I read the codes about compacting user mem maps. Currently, the > > function only merges adjacent user mem maps and does not remove the > same entry. > > > > How about removing the same entry in the fuction? > > I would've expected "the same" to be within the definition of "adjacent". Can > you confirm that this actually doesn't happen? If so, then yes, probably > compacting should do that, instead of relying on an artifact of > implementation.
OK, I will do that, will send the v2 later. Thanks Yunjian > > > > > Thanks > > Yunjian > > > >> > >> -- > >> Thanks, > >> Anatoly > > > -- > Thanks, > Anatoly
