From: "Yang, Yi" <yi.y.yang@intel.com>
To: "Tan, Jianfeng" <jianfeng.tan@intel.com>
Cc: "yliu@fridaylinux.org" <yliu@fridaylinux.org>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] vhost: fix vhost_user_set_mem_table error
Date: Mon, 25 Sep 2017 10:29:25 +0800 [thread overview]
Message-ID: <20170925022924.GA106755@localhost.localdomain> (raw)
In-Reply-To: <ED26CBA2FAD1BF48A8719AEF02201E36512EF802@SHSMSX103.ccr.corp.intel.com>
On Fri, Sep 22, 2017 at 01:28:48PM +0800, Tan, Jianfeng wrote:
>
>
> > + }
> > + reg->guest_user_addr = 0;
> > + j++;
> > + } else {
> > + i++;
> > + j++;
> > + }
> > + }
>
> The algorithm here could be problematic for hot plug. For example,
>
> T0: we have two regions with address 1, 3.
> T1: we add another region, 2, which makes the regions as 1, 2, 3.
>
> 1st iteration, 1 matches 1, i++, j++;
> 2nd iteration, 2 does not match 3, unmap 3, j++.
Jianfeng, thank you so much for your comments, but per my understanding
such hot add is impossible, this will result in discontinuous physical
address. Do you know how we can trigger such use case?
>
> > +
> > + /* munmap these regions because they have been hot
> > unplugged */
> > + for (; j < dev->mem->nregions; j++) {
> > + reg = &dev->mem->regions[j];
> > + if (reg->host_user_addr) {
> > + munmap(reg->mmap_addr, reg-
> > >mmap_size);
> > + close(reg->fd);
> > + }
> > + reg->guest_user_addr = 0;
> > + }
> > }
> >
> > - dev->nr_guest_pages = 0;
> > if (!dev->guest_pages) {
> > + dev->nr_guest_pages = 0;
>
> We cannot just move this line here, which results in a wrong nr_guest_pages if we do hot plug/unplug instead of initialization.
For initialization, dev->guest_pages is 0, for hot plug/unplug,
dev->guest_pages isn't 0, so it is ok.
>
> > dev->max_guest_pages = 8;
> > dev->guest_pages = malloc(dev->max_guest_pages *
> > sizeof(struct guest_page));
> > @@ -532,7 +558,7 @@ vhost_user_set_mem_table(struct virtio_net *dev,
> > struct VhostUserMsg *pmsg)
> > }
> > }
> >
> > - dev->mem = rte_zmalloc("vhost-mem-table", sizeof(struct
> > rte_vhost_memory) +
> > + dev->mem = rte_realloc(dev->mem, sizeof(struct
> > rte_vhost_memory) +
> > sizeof(struct rte_vhost_mem_region) * memory.nregions, 0);
> > if (dev->mem == NULL) {
> > RTE_LOG(ERR, VHOST_CONFIG,
> > @@ -546,6 +572,38 @@ vhost_user_set_mem_table(struct virtio_net *dev,
> > struct VhostUserMsg *pmsg)
> > fd = pmsg->fds[i];
> > reg = &dev->mem->regions[i];
> >
> > + /* This region should be skipped if it is initialized before */
> > + if (reg->guest_user_addr ==
> > memory.regions[i].userspace_addr) {
>
> Also problematic for hot unplug. For example,
> T0: we have three regions, 1, 2, 3.
> T1: remove region 2, with only 1, 3 left.
>
> 1st iteration, 1 matches 1, skip old region 1.
> 2nd iteration, 3 does not match 2, do the mmap.
It is ok, I have verified it, let us have a discussion in meeting.
next prev parent reply other threads:[~2017-09-25 2:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-20 8:32 Yi Yang
2017-09-22 5:28 ` Tan, Jianfeng
2017-09-25 2:29 ` Yang, Yi [this message]
2017-09-25 4:46 ` Tiwei Bie
Reply instructions:
You may reply publicly 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=20170925022924.GA106755@localhost.localdomain \
--to=yi.y.yang@intel.com \
--cc=dev@dpdk.org \
--cc=jianfeng.tan@intel.com \
--cc=yliu@fridaylinux.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).