DPDK patches and discussions
 help / color / Atom feed
From: wangyunjian <wangyunjian@huawei.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>
Cc: "Lilijun (Jerry)" <jerry.lilijun@huawei.com>,
	xudingke <xudingke@huawei.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/1] eal/linux: do not create user mem map repeatedly when it exists
Date: Wed, 22 Jul 2020 12:47:45 +0000
Message-ID: <34EFBCA9F01B0748BEB6B629CE643AE60D0DD75E@DGGEMM533-MBX.china.huawei.com> (raw)
In-Reply-To: <332ede30-4dae-1fde-b288-25f29469f3e7@intel.com>

> -----Original Message-----
> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> Sent: Monday, July 20, 2020 7:46 PM
> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org;
> david.marchand@redhat.com
> Cc: Lilijun (Jerry) <jerry.lilijun@huawei.com>; xudingke
> <xudingke@huawei.com>; stable@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.burakov@intel.com]
> >> Sent: Friday, July 17, 2020 10:24 PM
> >> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org;
> >> david.marchand@redhat.com
> >> Cc: Lilijun (Jerry) <jerry.lilijun@huawei.com>; xudingke
> >> <xudingke@huawei.com>; stable@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 <wangyunjian@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: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Yunjian Wang <wangyunjian@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

  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 13:38 wangyunjian
2020-07-17 14:19 ` Burakov, Anatoly
2020-07-17 14:23   ` Burakov, Anatoly
2020-07-20  2:00     ` wangyunjian
2020-07-20 11:46       ` Burakov, Anatoly
2020-07-22 12:47         ` wangyunjian [this message]
2020-07-23 14:48 ` [dpdk-dev] [PATCH v2] " wangyunjian
2020-07-24 13:25   ` Burakov, Anatoly
2020-07-25  9:59     ` wangyunjian
2020-07-27  9:24       ` Burakov, Anatoly
2020-07-30 13:16         ` wangyunjian
2020-07-31 11:55           ` Burakov, Anatoly
2020-08-05 12:58             ` wangyunjian
2020-09-17 11:33               ` Burakov, Anatoly
2020-09-17 11:35   ` Burakov, Anatoly

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=34EFBCA9F01B0748BEB6B629CE643AE60D0DD75E@DGGEMM533-MBX.china.huawei.com \
    --to=wangyunjian@huawei.com \
    --cc=anatoly.burakov@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=jerry.lilijun@huawei.com \
    --cc=stable@dpdk.org \
    --cc=xudingke@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox