* RE: [PATCH v9 5/5] virtio-balloon: VIRTIO_BALLOON_F_MISC_VQ
From: Wang, Wei W @ 2017-05-07 4:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, Hansen, Dave,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <20170506011928-mutt-send-email-mst@kernel.org>
On 05/06/2017 06:21 AM, Michael S. Tsirkin wrote:
> On Thu, Apr 27, 2017 at 02:33:44PM +0800, Wei Wang wrote:
> > On 04/14/2017 01:08 AM, Michael S. Tsirkin wrote:
> > > On Thu, Apr 13, 2017 at 05:35:08PM +0800, Wei Wang wrote:
> > > > Add a new vq, miscq, to handle miscellaneous requests between the
> > > > device and the driver.
> > > >
> > > > This patch implemnts the
> VIRTIO_BALLOON_MISCQ_INQUIRE_UNUSED_PAGES
> > > implements
> > >
> > > > request sent from the device.
> > > Commands are sent from host and handled on guest.
> > > In fact how is this so different from stats?
> > > How about reusing the stats vq then? You can use one buffer for
> > > stats and one buffer for commands.
> > >
> >
> > The meaning of the two vqs is a little different. statq is used for
> > reporting statistics, while miscq is intended to be used to handle
> > miscellaneous requests from the guest or host
>
> misc just means "anything goes". If you want it to mean "commands" name it so.
Ok, will change it.
> > (I think it can
> > also be used the other way around in the future when other new
> > features are added which need the guest to send requests and the host
> > to provide responses).
> >
> > I would prefer to have them separate, because:
> > If we plan to combine them, we need to put the previous statq related
> > implementation under miscq with a new command (I think we can't
> > combine them without using commands to distinguish the two features).
>
> Right.
> > In this way, an old driver won't work with a new QEMU or a new driver
> > won't work with an old QEMU. Would this be considered as an issue
> > here?
>
> Compatibility is and should always be handled using feature flags. There's a
> feature flag for this, isn't it?
The negotiation of the existing feature flag, VIRTIO_BALLOON_F_STATS_VQ
only indicates the support of the old statq implementation. To move the statq
implementation under cmdq, I think we would need a new feature flag for the
new statq implementation:
#define VIRTIO_BALLOON_F_CMDQ_STATS 5
What do you think?
Best,
Wei
^ permalink raw reply
* RE: [virtio-dev] Re: [PATCH v9 2/5] virtio-balloon: VIRTIO_BALLOON_F_BALLOON_CHUNKS
From: Wang, Wei W @ 2017-05-07 4:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, Hansen, Dave,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <20170506012322-mutt-send-email-mst@kernel.org>
On 05/06/2017 06:26 AM, Michael S. Tsirkin wrote:
> On Thu, Apr 27, 2017 at 02:31:49PM +0800, Wei Wang wrote:
> > On 04/27/2017 07:20 AM, Michael S. Tsirkin wrote:
> > > On Wed, Apr 26, 2017 at 11:03:34AM +0000, Wang, Wei W wrote:
> > > > Hi Michael, could you please give some feedback?
> > > I'm sorry, I'm not sure feedback on what you are requesting.
> > Oh, just some trivial things (e.g. use a field in the header,
> > hdr->chunks to indicate the number of chunks in the payload) that
> > wasn't confirmed.
> >
> > I will prepare the new version with fixing the agreed issues, and we
> > can continue to discuss those parts if you still find them improper.
> >
> >
> > >
> > > The interface looks reasonable now, even though there's a way to
> > > make it even simpler if we can limit chunk size to 2G (in fact 4G -
> > > 1). Do you think we can live with this limitation?
> > Yes, I think we can. So, is it good to change to use the previous
> > 64-bit chunk format (52-bit base + 12-bit size)?
>
> This isn't what I meant. virtio ring has descriptors with a 64 bit address and 32 bit
> size.
>
> If size < 4g is not a significant limitation, why not just use that to pass
> address/size in a standard s/g list, possibly using INDIRECT?
OK, I see your point, thanks. Post the two options here for an analysis:
Option1 (what we have now):
struct virtio_balloon_page_chunk {
__le64 chunk_num;
struct virtio_balloon_page_chunk_entry entry[];
};
Option2:
struct virtio_balloon_page_chunk {
__le64 chunk_num;
struct scatterlist entry[];
};
I don't have an issue to change it to Option2, but I would prefer Option1,
because I think there is no be obvious difference between the two options,
while Option1 appears to have little advantages here:
1) "struct virtio_balloon_page_chunk_entry" has smaller size than
"struct scatterlist", so the same size of allocated page chunk buffer
can hold more entry[] using Option1;
2) INDIRECT needs on demand kmalloc();
3) no 4G size limit;
What do you think?
Best,
Wei
^ permalink raw reply
* Re: [PATCH v10 3/6] virtio-balloon: VIRTIO_BALLOON_F_PAGE_CHUNKS
From: Michael S. Tsirkin @ 2017-05-05 22:29 UTC (permalink / raw)
To: Wei Wang
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, dave.hansen, linux-kernel, virtualization,
linux-mm, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <1493887815-6070-4-git-send-email-wei.w.wang@intel.com>
On Thu, May 04, 2017 at 04:50:12PM +0800, Wei Wang wrote:
> Add a new feature, VIRTIO_BALLOON_F_PAGE_CHUNKS, which enables
> the transfer of the ballooned (i.e. inflated/deflated) pages in
> chunks to the host.
>
> The implementation of the previous virtio-balloon is not very
> efficient, because the ballooned pages are transferred to the
> host one by one. Here is the breakdown of the time in percentage
> spent on each step of the balloon inflating process (inflating
> 7GB of an 8GB idle guest).
>
> 1) allocating pages (6.5%)
> 2) sending PFNs to host (68.3%)
> 3) address translation (6.1%)
> 4) madvise (19%)
>
> It takes about 4126ms for the inflating process to complete.
> The above profiling shows that the bottlenecks are stage 2)
> and stage 4).
>
> This patch optimizes step 2) by transferring pages to the host in
> chunks. A chunk consists of guest physically continuous pages.
> When the pages are packed into a chunk, they are converted into
> balloon page size (4KB) pages. A chunk is offered to the host
> via a base PFN (i.e. the start PFN of those physically continuous
> pages) and the size (i.e. the total number of the 4KB balloon size
> pages). A chunk is formatted as below:
> --------------------------------------------------------
> | Base (52 bit) | Rsvd (12 bit) |
> --------------------------------------------------------
> --------------------------------------------------------
> | Size (52 bit) | Rsvd (12 bit) |
> --------------------------------------------------------
>
> By doing so, step 4) can also be optimized by doing address
> translation and madvise() in chunks rather than page by page.
>
> With this new feature, the above ballooning process takes ~590ms
> resulting in an improvement of ~85%.
>
> TODO: optimize stage 1) by allocating/freeing a chunk of pages
> instead of a single page each time.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
This is much cleaner, thanks. It might be even better to
have wrappers that put array and its size in a struct
and manage that struct, but I won't require this for
submission.
> ---
> drivers/virtio/virtio_balloon.c | 407 +++++++++++++++++++++++++++++++++---
> include/uapi/linux/virtio_balloon.h | 14 ++
> 2 files changed, 396 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index ecb64e9..df16912 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -43,6 +43,20 @@
> #define OOM_VBALLOON_DEFAULT_PAGES 256
> #define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
>
> +/* The size of one page_bmap used to record inflated/deflated pages. */
> +#define VIRTIO_BALLOON_PAGE_BMAP_SIZE (8 * PAGE_SIZE)
> +/*
> + * Callulates how many pfns can a page_bmap record. A bit corresponds to a
> + * page of PAGE_SIZE.
> + */
> +#define VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP \
> + (VIRTIO_BALLOON_PAGE_BMAP_SIZE * BITS_PER_BYTE)
> +
> +/* The number of page_bmap to allocate by default. */
> +#define VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM 1
> +/* The maximum number of page_bmap that can be allocated. */
> +#define VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM 32
> +
> static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
> module_param(oom_pages, int, S_IRUSR | S_IWUSR);
> MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
> @@ -51,6 +65,11 @@ MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
> static struct vfsmount *balloon_mnt;
> #endif
>
> +/* Maximum number of page chunks */
> +#define VIRTIO_BALLOON_MAX_PAGE_CHUNKS ((8 * PAGE_SIZE - \
> + sizeof(struct virtio_balloon_page_chunk)) / \
> + sizeof(struct virtio_balloon_page_chunk_entry))
> +
> struct virtio_balloon {
> struct virtio_device *vdev;
> struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> @@ -79,6 +98,12 @@ struct virtio_balloon {
> /* Synchronize access/update to this struct virtio_balloon elements */
> struct mutex balloon_lock;
>
> + /* Buffer for chunks of ballooned pages. */
> + struct virtio_balloon_page_chunk *balloon_page_chunk;
> +
> + /* Bitmap used to record pages. */
> + unsigned long *page_bmap[VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM];
> +
> /* The array of pfns we tell the Host about. */
> unsigned int num_pfns;
> __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
> @@ -111,6 +136,136 @@ static void balloon_ack(struct virtqueue *vq)
> wake_up(&vb->acked);
> }
>
> +/* Update pfn_max and pfn_min according to the pfn of page */
> +static inline void update_pfn_range(struct virtio_balloon *vb,
> + struct page *page,
> + unsigned long *pfn_min,
> + unsigned long *pfn_max)
> +{
> + unsigned long pfn = page_to_pfn(page);
> +
> + *pfn_min = min(pfn, *pfn_min);
> + *pfn_max = max(pfn, *pfn_max);
> +}
> +
> +static unsigned int extend_page_bmap_size(struct virtio_balloon *vb,
> + unsigned long pfn_num)
> +{
> + unsigned int i, bmap_num, allocated_bmap_num;
> + unsigned long bmap_len;
> +
> + allocated_bmap_num = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM;
> + bmap_len = ALIGN(pfn_num, BITS_PER_LONG) / BITS_PER_BYTE;
> + bmap_len = roundup(bmap_len, VIRTIO_BALLOON_PAGE_BMAP_SIZE);
> + /*
> + * VIRTIO_BALLOON_PAGE_BMAP_SIZE is the size of one page_bmap, so
> + * divide it to calculate how many page_bmap that we need.
> + */
> + bmap_num = (unsigned int)(bmap_len / VIRTIO_BALLOON_PAGE_BMAP_SIZE);
> + /* The number of page_bmap
... arrays ...
> to allocate should not exceed the max */
> + bmap_num = min_t(unsigned int, VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM,
> + bmap_num);
> +
> + for (i = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i < bmap_num; i++) {
> + vb->page_bmap[i] = kmalloc(VIRTIO_BALLOON_PAGE_BMAP_SIZE,
> + GFP_KERNEL);
> + if (vb->page_bmap[i])
> + allocated_bmap_num++;
> + else
> + break;
> + }
> +
> + return allocated_bmap_num;
> +}
> +
> +static void free_extended_page_bmap(struct virtio_balloon *vb,
> + unsigned int page_bmap_num)
> +{
> + unsigned int i;
> +
> + for (i = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i < page_bmap_num;
> + i++) {
> + kfree(vb->page_bmap[i]);
> + vb->page_bmap[i] = NULL;
> + page_bmap_num--;
> + }
> +}
> +
> +static void clear_page_bmap(struct virtio_balloon *vb,
> + unsigned int page_bmap_num)
> +{
> + int i;
> +
> + for (i = 0; i < page_bmap_num; i++)
> + memset(vb->page_bmap[i], 0, VIRTIO_BALLOON_PAGE_BMAP_SIZE);
> +}
> +
> +static void send_page_chunks(struct virtio_balloon *vb, struct virtqueue *vq)
> +{
> + struct scatterlist sg;
> + struct virtio_balloon_page_chunk *chunk;
> + unsigned int len;
> +
> + chunk = vb->balloon_page_chunk;
> + len = sizeof(__le64) +
> + le64_to_cpu(chunk->chunk_num) *
> + sizeof(struct virtio_balloon_page_chunk_entry);
> + sg_init_one(&sg, chunk, len);
> + if (!virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL)) {
> + virtqueue_kick(vq);
> + wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> + chunk->chunk_num = 0;
> + }
> +}
> +
> +/* Add a chunk entry to the buffer. */
> +static void add_one_chunk(struct virtio_balloon *vb, struct virtqueue *vq,
> + u64 base, u64 size)
> +{
> + struct virtio_balloon_page_chunk *chunk = vb->balloon_page_chunk;
> + struct virtio_balloon_page_chunk_entry *entry;
> + uint64_t chunk_num = le64_to_cpu(chunk->chunk_num);
> +
> + entry = &chunk->entry[chunk_num];
> + entry->base = cpu_to_le64(base << VIRTIO_BALLOON_CHUNK_BASE_SHIFT);
> + entry->size = cpu_to_le64(size << VIRTIO_BALLOON_CHUNK_SIZE_SHIFT);
> + chunk->chunk_num = cpu_to_le64(++chunk_num);
> + if (chunk_num == VIRTIO_BALLOON_MAX_PAGE_CHUNKS)
> + send_page_chunks(vb, vq);
> +}
> +
> +static void convert_bmap_to_chunks(struct virtio_balloon *vb,
> + struct virtqueue *vq,
> + unsigned long *bmap,
> + unsigned long pfn_start,
> + unsigned long size)
> +{
> + unsigned long next_one, next_zero, chunk_size, pos = 0;
> +
> + while (pos < size) {
> + next_one = find_next_bit(bmap, size, pos);
> + /*
> + * No "1" bit found, which means that there is no pfn
> + * recorded in the rest of this bmap.
> + */
> + if (next_one == size)
> + break;
> + next_zero = find_next_zero_bit(bmap, size, next_one + 1);
> + /*
> + * A bit in page_bmap corresponds to a page of PAGE_SIZE.
> + * Convert it to be pages of 4KB balloon page size when
> + * adding it to a chunk.
> + */
> + chunk_size = (next_zero - next_one) *
> + VIRTIO_BALLOON_PAGES_PER_PAGE;
> + if (chunk_size) {
> + add_one_chunk(vb, vq, pfn_start + next_one,
> + chunk_size);
> + pos += next_zero + 1;
> + }
> + }
> +}
> +
> static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
> {
> struct scatterlist sg;
> @@ -124,7 +279,33 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
>
> /* When host has read buffer, this completes via balloon_ack */
> wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> +}
> +
> +static void tell_host_from_page_bmap(struct virtio_balloon *vb,
> + struct virtqueue *vq,
> + unsigned long pfn_start,
> + unsigned long pfn_end,
> + unsigned int page_bmap_num)
> +{
> + unsigned long i, pfn_num;
>
> + for (i = 0; i < page_bmap_num; i++) {
> + /*
> + * For the last page_bmap, only the remaining number of pfns
> + * need to be searched rather than the entire page_bmap.
> + */
> + if (i + 1 == page_bmap_num)
> + pfn_num = (pfn_end - pfn_start) %
> + VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
> + else
> + pfn_num = VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
> +
> + convert_bmap_to_chunks(vb, vq, vb->page_bmap[i], pfn_start +
> + i * VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP,
> + pfn_num);
> + }
> + if (le64_to_cpu(vb->balloon_page_chunk->chunk_num) > 0)
> + send_page_chunks(vb, vq);
> }
>
> static void set_page_pfns(struct virtio_balloon *vb,
> @@ -141,13 +322,88 @@ static void set_page_pfns(struct virtio_balloon *vb,
> page_to_balloon_pfn(page) + i);
> }
>
> +/*
> + * Send ballooned pages in chunks to host.
> + * The ballooned pages are recorded in page bitmaps. Each bit in a bitmap
> + * corresponds to a page of PAGE_SIZE. The page bitmaps are searched for
> + * continuous "1" bits, which correspond to continuous pages, to chunk.
> + * When packing those continuous pages into chunks, pages are converted into
> + * 4KB balloon pages.
> + *
> + * pfn_max and pfn_min form the range of pfns that need to use page bitmaps to
> + * record. If the range is too large to be recorded into the allocated page
> + * bitmaps, the page bitmaps are used multiple times to record the entire
> + * range of pfns.
> + */
> +static void tell_host_page_chunks(struct virtio_balloon *vb,
> + struct list_head *pages,
> + struct virtqueue *vq,
> + unsigned long pfn_max,
> + unsigned long pfn_min)
> +{
> + /*
> + * The pfn_start and pfn_end form the range of pfns that the allocated
> + * page_bmap can record in each round.
> + */
> + unsigned long pfn_start, pfn_end;
> + /* Total number of allocated page_bmap
arrays
> */
> + unsigned int page_bmap_num;
> + struct page *page;
> + bool found;
> +
> + /*
> + * In the case that one page_bmap is not sufficient to record the pfn
> + * range, page_bmap will be extended by allocating more numbers of
> + * page_bmap.
> + */
> + page_bmap_num = extend_page_bmap_size(vb, pfn_max - pfn_min + 1);
> +
> + /* Start from the beginning of the whole pfn range */
> + pfn_start = pfn_min;
> + while (pfn_start < pfn_max) {
> + pfn_end = pfn_start +
> + VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP * page_bmap_num;
> + pfn_end = pfn_end < pfn_max ? pfn_end : pfn_max;
> + clear_page_bmap(vb, page_bmap_num);
> + found = false;
> +
> + list_for_each_entry(page, pages, lru) {
> + unsigned long bmap_idx, bmap_pos, this_pfn;
> +
> + this_pfn = page_to_pfn(page);
> + if (this_pfn < pfn_start || this_pfn > pfn_end)
> + continue;
> + bmap_idx = (this_pfn - pfn_start) /
> + VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
> + bmap_pos = (this_pfn - pfn_start) %
> + VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
> + set_bit(bmap_pos, vb->page_bmap[bmap_idx]);
> +
> + found = true;
> + }
> + if (found)
> + tell_host_from_page_bmap(vb, vq, pfn_start, pfn_end,
> + page_bmap_num);
> + /*
> + * Start the next round when pfn_start and pfn_end couldn't
> + * cover the whole pfn range given by pfn_max and pfn_min.
> + */
> + pfn_start = pfn_end;
> + }
> + free_extended_page_bmap(vb, page_bmap_num);
> +}
> +
> static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> {
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> unsigned num_allocated_pages;
> + bool chunking = virtio_has_feature(vb->vdev,
> + VIRTIO_BALLOON_F_PAGE_CHUNKS);
> + unsigned long pfn_max = 0, pfn_min = ULONG_MAX;
>
> /* We can only do one array worth at a time. */
> - num = min(num, ARRAY_SIZE(vb->pfns));
> + if (!chunking)
> + num = min(num, ARRAY_SIZE(vb->pfns));
>
> mutex_lock(&vb->balloon_lock);
> for (vb->num_pfns = 0; vb->num_pfns < num;
> @@ -162,7 +418,10 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> msleep(200);
> break;
> }
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + if (chunking)
> + update_pfn_range(vb, page, &pfn_max, &pfn_min);
> + else
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
> if (!virtio_has_feature(vb->vdev,
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> @@ -171,8 +430,14 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>
> num_allocated_pages = vb->num_pfns;
> /* Did we get any? */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->inflate_vq);
> + if (vb->num_pfns != 0) {
> + if (chunking)
> + tell_host_page_chunks(vb, &vb_dev_info->pages,
> + vb->inflate_vq,
> + pfn_max, pfn_min);
> + else
> + tell_host(vb, vb->inflate_vq);
> + }
> mutex_unlock(&vb->balloon_lock);
>
> return num_allocated_pages;
> @@ -198,9 +463,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> struct page *page;
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> LIST_HEAD(pages);
> + bool chunking = virtio_has_feature(vb->vdev,
> + VIRTIO_BALLOON_F_PAGE_CHUNKS);
> + unsigned long pfn_max = 0, pfn_min = ULONG_MAX;
>
> - /* We can only do one array worth at a time. */
> - num = min(num, ARRAY_SIZE(vb->pfns));
> + /* Traditionally, we can only do one array worth at a time. */
> + if (!chunking)
> + num = min(num, ARRAY_SIZE(vb->pfns));
>
> mutex_lock(&vb->balloon_lock);
> /* We can't release more pages than taken */
> @@ -210,7 +479,10 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> page = balloon_page_dequeue(vb_dev_info);
> if (!page)
> break;
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + if (chunking)
> + update_pfn_range(vb, page, &pfn_max, &pfn_min);
> + else
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> list_add(&page->lru, &pages);
> vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
> }
> @@ -221,8 +493,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> * virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
> * is true, we *have* to do it in this order
> */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->deflate_vq);
> + if (vb->num_pfns != 0) {
> + if (chunking)
> + tell_host_page_chunks(vb, &pages, vb->deflate_vq,
> + pfn_max, pfn_min);
> + else
> + tell_host(vb, vb->deflate_vq);
> + }
> release_pages_balloon(vb, &pages);
> mutex_unlock(&vb->balloon_lock);
> return num_freed_pages;
> @@ -442,6 +719,14 @@ static int init_vqs(struct virtio_balloon *vb)
> }
>
> #ifdef CONFIG_BALLOON_COMPACTION
> +
> +static void tell_host_one_page(struct virtio_balloon *vb,
> + struct virtqueue *vq, struct page *page)
> +{
> + add_one_chunk(vb, vq, page_to_pfn(page),
> + VIRTIO_BALLOON_PAGES_PER_PAGE);
> +}
> +
> /*
> * virtballoon_migratepage - perform the balloon page migration on behalf of
> * a compation thread. (called under page lock)
> @@ -465,6 +750,8 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
> {
> struct virtio_balloon *vb = container_of(vb_dev_info,
> struct virtio_balloon, vb_dev_info);
> + bool chunking = virtio_has_feature(vb->vdev,
> + VIRTIO_BALLOON_F_PAGE_CHUNKS);
> unsigned long flags;
>
> /*
> @@ -486,16 +773,22 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
> vb_dev_info->isolated_pages--;
> __count_vm_event(BALLOON_MIGRATE);
> spin_unlock_irqrestore(&vb_dev_info->pages_lock, flags);
> - vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
> - set_page_pfns(vb, vb->pfns, newpage);
> - tell_host(vb, vb->inflate_vq);
> -
> + if (chunking) {
> + tell_host_one_page(vb, vb->inflate_vq, newpage);
> + } else {
> + vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
> + set_page_pfns(vb, vb->pfns, newpage);
> + tell_host(vb, vb->inflate_vq);
> + }
> /* balloon's page migration 2nd step -- deflate "page" */
> balloon_page_delete(page);
> - vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
> - set_page_pfns(vb, vb->pfns, page);
> - tell_host(vb, vb->deflate_vq);
> -
> + if (chunking) {
> + tell_host_one_page(vb, vb->deflate_vq, page);
> + } else {
> + vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
> + set_page_pfns(vb, vb->pfns, page);
> + tell_host(vb, vb->deflate_vq);
> + }
> mutex_unlock(&vb->balloon_lock);
>
> put_page(page); /* balloon reference */
> @@ -522,9 +815,75 @@ static struct file_system_type balloon_fs = {
>
> #endif /* CONFIG_BALLOON_COMPACTION */
>
> +static void free_page_bmap(struct virtio_balloon *vb)
> +{
> + int i;
> +
> + for (i = 0; i < VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i++) {
> + kfree(vb->page_bmap[i]);
> + vb->page_bmap[i] = NULL;
> + }
> +}
> +
> +static int balloon_page_chunk_init(struct virtio_balloon *vb)
> +{
> + int i;
> +
> + vb->balloon_page_chunk = kmalloc(sizeof(__le64) +
> + sizeof(struct virtio_balloon_page_chunk_entry) *
> + VIRTIO_BALLOON_MAX_PAGE_CHUNKS, GFP_KERNEL);
> + if (!vb->balloon_page_chunk)
> + goto err_page_chunk;
> +
> + /*
> + * The default number of page_bmaps are allocated. More may be
> + * allocated on demand.
> + */
> + for (i = 0; i < VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i++) {
> + vb->page_bmap[i] = kmalloc(VIRTIO_BALLOON_PAGE_BMAP_SIZE,
> + GFP_KERNEL);
> + if (!vb->page_bmap[i])
> + goto err_page_bmap;
> + }
> +
> + return 0;
> +err_page_bmap:
> + free_page_bmap(vb);
> + kfree(vb->balloon_page_chunk);
> + vb->balloon_page_chunk = NULL;
> +err_page_chunk:
> + __virtio_clear_bit(vb->vdev, VIRTIO_BALLOON_F_PAGE_CHUNKS);
> + dev_warn(&vb->vdev->dev, "%s: failed\n", __func__);
> + return -ENOMEM;
> +}
> +
> +static int virtballoon_validate(struct virtio_device *vdev)
> +{
> + struct virtio_balloon *vb = NULL;
> + int err;
> +
> + vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
> + if (!vb) {
> + err = -ENOMEM;
> + goto err_vb;
> + }
> +
> + if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_CHUNKS)) {
> + err = balloon_page_chunk_init(vb);
> + if (err < 0)
> + goto err_page_chunk;
> + }
> +
> + return 0;
> +err_page_chunk:
> + kfree(vb);
> +err_vb:
> + return err;
> +}
> +
> static int virtballoon_probe(struct virtio_device *vdev)
> {
> - struct virtio_balloon *vb;
> + struct virtio_balloon *vb = vdev->priv;
> int err;
>
> if (!vdev->config->get) {
> @@ -533,17 +892,12 @@ static int virtballoon_probe(struct virtio_device *vdev)
> return -EINVAL;
> }
>
> - vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
> - if (!vb) {
> - err = -ENOMEM;
> - goto out;
> - }
> -
> INIT_WORK(&vb->update_balloon_stats_work, update_balloon_stats_func);
> INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
> spin_lock_init(&vb->stop_update_lock);
> vb->stop_update = false;
> vb->num_pages = 0;
> +
> mutex_init(&vb->balloon_lock);
> init_waitqueue_head(&vb->acked);
> vb->vdev = vdev;
> @@ -590,7 +944,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
> vdev->config->del_vqs(vdev);
> out_free_vb:
> kfree(vb);
> -out:
> return err;
> }
>
> @@ -620,6 +973,8 @@ static void virtballoon_remove(struct virtio_device *vdev)
> cancel_work_sync(&vb->update_balloon_stats_work);
>
> remove_common(vb);
> + free_page_bmap(vb);
> + kfree(vb->balloon_page_chunk);
> #ifdef CONFIG_BALLOON_COMPACTION
> if (vb->vb_dev_info.inode)
> iput(vb->vb_dev_info.inode);
> @@ -664,6 +1019,7 @@ static unsigned int features[] = {
> VIRTIO_BALLOON_F_MUST_TELL_HOST,
> VIRTIO_BALLOON_F_STATS_VQ,
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
> + VIRTIO_BALLOON_F_PAGE_CHUNKS,
> };
>
> static struct virtio_driver virtio_balloon_driver = {
> @@ -674,6 +1030,7 @@ static struct virtio_driver virtio_balloon_driver = {
> .id_table = id_table,
> .probe = virtballoon_probe,
> .remove = virtballoon_remove,
> + .validate = virtballoon_validate,
> .config_changed = virtballoon_changed,
> #ifdef CONFIG_PM_SLEEP
> .freeze = virtballoon_freeze,
> diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
> index 343d7dd..d532ed16 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -34,6 +34,7 @@
> #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
> #define VIRTIO_BALLOON_F_STATS_VQ 1 /* Memory Stats virtqueue */
> #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
> +#define VIRTIO_BALLOON_F_PAGE_CHUNKS 3 /* Inflate/Deflate pages in chunks */
>
> /* Size of a PFN in the balloon interface. */
> #define VIRTIO_BALLOON_PFN_SHIFT 12
> @@ -82,4 +83,17 @@ struct virtio_balloon_stat {
> __virtio64 val;
> } __attribute__((packed));
>
> +#define VIRTIO_BALLOON_CHUNK_BASE_SHIFT 12
> +#define VIRTIO_BALLOON_CHUNK_SIZE_SHIFT 12
> +struct virtio_balloon_page_chunk_entry {
> + __le64 base;
> + __le64 size;
> +};
> +
> +struct virtio_balloon_page_chunk {
> + /* Number of chunks in the payload */
> + __le64 chunk_num;
> + struct virtio_balloon_page_chunk_entry entry[];
> +};
> +
> #endif /* _LINUX_VIRTIO_BALLOON_H */
> --
> 2.7.4
^ permalink raw reply
* Re: [virtio-dev] Re: [PATCH v9 2/5] virtio-balloon: VIRTIO_BALLOON_F_BALLOON_CHUNKS
From: Michael S. Tsirkin @ 2017-05-05 22:25 UTC (permalink / raw)
To: Wei Wang
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, Hansen, Dave,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <59019055.3040708@intel.com>
On Thu, Apr 27, 2017 at 02:31:49PM +0800, Wei Wang wrote:
> On 04/27/2017 07:20 AM, Michael S. Tsirkin wrote:
> > On Wed, Apr 26, 2017 at 11:03:34AM +0000, Wang, Wei W wrote:
> > > Hi Michael, could you please give some feedback?
> > I'm sorry, I'm not sure feedback on what you are requesting.
> Oh, just some trivial things (e.g. use a field in the
> header, hdr->chunks to indicate the number of chunks
> in the payload) that wasn't confirmed.
>
> I will prepare the new version with fixing the agreed issues,
> and we can continue to discuss those parts if you still find
> them improper.
>
>
> >
> > The interface looks reasonable now, even though there's
> > a way to make it even simpler if we can limit chunk size
> > to 2G (in fact 4G - 1). Do you think we can live with this
> > limitation?
> Yes, I think we can. So, is it good to change to use the
> previous 64-bit chunk format (52-bit base + 12-bit size)?
This isn't what I meant. virtio ring has descriptors with
a 64 bit address and 32 bit size.
If size < 4g is not a significant limitation, why not just
use that to pass address/size in a standard s/g list,
possibly using INDIRECT?
>
> >
> > But the code still needs some cleanup.
> >
>
> OK. We'll also still to discuss your comments in the patch 05.
>
> Best,
> Wei
^ permalink raw reply
* Re: [PATCH v9 5/5] virtio-balloon: VIRTIO_BALLOON_F_MISC_VQ
From: Michael S. Tsirkin @ 2017-05-05 22:21 UTC (permalink / raw)
To: Wei Wang
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, dave.hansen, linux-kernel, virtualization,
linux-mm, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <590190C8.6030609@intel.com>
On Thu, Apr 27, 2017 at 02:33:44PM +0800, Wei Wang wrote:
> On 04/14/2017 01:08 AM, Michael S. Tsirkin wrote:
> > On Thu, Apr 13, 2017 at 05:35:08PM +0800, Wei Wang wrote:
> > > Add a new vq, miscq, to handle miscellaneous requests between the device
> > > and the driver.
> > >
> > > This patch implemnts the VIRTIO_BALLOON_MISCQ_INQUIRE_UNUSED_PAGES
> > implements
> >
> > > request sent from the device.
> > Commands are sent from host and handled on guest.
> > In fact how is this so different from stats?
> > How about reusing the stats vq then? You can use one buffer
> > for stats and one buffer for commands.
> >
>
> The meaning of the two vqs is a little different. statq is used for
> reporting statistics, while miscq is intended to be used to handle
> miscellaneous requests from the guest or host
misc just means "anything goes". If you want it to mean
"commands" name it so.
> (I think it can
> also be used the other way around in the future when other
> new features are added which need the guest to send requests
> and the host to provide responses).
>
> I would prefer to have them separate, because:
> If we plan to combine them, we need to put the previous statq
> related implementation under miscq with a new command (I think
> we can't combine them without using commands to distinguish
> the two features).
Right.
> In this way, an old driver won't work with a new QEMU or a new
> driver won't work with an old QEMU. Would this be considered
> as an issue here?
Compatibility is and should always be handled using
feature flags. There's a feature flag for this, isn't it?
>
>
> >
> > > + miscq_out_hdr->flags = 0;
> > > +
> > > + for_each_populated_zone(zone) {
> > > + for (order = MAX_ORDER - 1; order > 0; order--) {
> > > + for (migratetype = 0; migratetype < MIGRATE_TYPES;
> > > + migratetype++) {
> > > + do {
> > > + ret = inquire_unused_page_block(zone,
> > > + order, migratetype, &page);
> > > + if (!ret) {
> > > + pfn = (u64)page_to_pfn(page);
> > > + add_one_chunk(vb, vq,
> > > + PAGE_CHUNK_TYPE_UNUSED,
> > > + pfn,
> > > + (u64)(1 << order));
> > > + }
> > > + } while (!ret);
> > > + }
> > > + }
> > > + }
> > > + miscq_out_hdr->flags |= VIRTIO_BALLOON_MISCQ_F_COMPLETE;
> > And where is miscq_out_hdr used? I see no add_outbuf anywhere.
> >
> > Things like this should be passed through function parameters
> > and not stuffed into device structure, fields should be
> > initialized before use and not where we happen to
> > have the data handy.
> >
>
> miscq_out_hdr is linear with the payload (i.e. kmalloc(hdr+payload) ).
> It is the same as the use of statq - one request in-flight each time.
>
>
> >
> > Also, _F_ is normally a bit number, you use it as a value here.
> >
> It intends to be a bit number. Bit 0 of flags to indicate the completion
> of handling the request.
^ permalink raw reply
* Re: [PATCH v10 4/6] mm: function to offer a page block on the free list
From: Michael S. Tsirkin @ 2017-05-05 22:12 UTC (permalink / raw)
To: kbuild test robot
Cc: aarcange, virtio-dev, amit.shah, kvm, linux-kernel,
liliang.opensource, dave.hansen, qemu-devel, virtualization,
linux-mm, kbuild-all, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <201705050851.KJdDIPUA%fengguang.wu@intel.com>
On Fri, May 05, 2017 at 08:21:34AM +0800, kbuild test robot wrote:
> Hi Wei,
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v4.11 next-20170504]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Wei-Wang/Extend-virtio-balloon-for-fast-de-inflating-fast-live-migration/20170505-052958
> reproduce: make htmldocs
>
> All warnings (new ones prefixed by >>):
>
> WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
> arch/x86/include/asm/uaccess_32.h:1: warning: no structured comments found
> >> mm/page_alloc.c:4663: warning: No description found for parameter 'zone'
> >> mm/page_alloc.c:4663: warning: No description found for parameter 'order'
> >> mm/page_alloc.c:4663: warning: No description found for parameter 'migratetype'
> >> mm/page_alloc.c:4663: warning: No description found for parameter 'page'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
> include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
> include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
>
> vim +/zone +4663 mm/page_alloc.c
the issue is actually aboe this line: it is:
/**
which is reserved for kernel-doc.
Either format properly for kernel-doc, or use simple /*
to start comments.
> 4647 * Heuristically get a page block in the system that is unused.
> 4648 * It is possible that pages from the page block are used immediately after
> 4649 * report_unused_page_block() returns. It is the caller's responsibility
> 4650 * to either detect or prevent the use of such pages.
> 4651 *
> 4652 * The free list to check: zone->free_area[order].free_list[migratetype].
> 4653 *
> 4654 * If the caller supplied page block (i.e. **page) is on the free list, offer
> 4655 * the next page block on the list to the caller. Otherwise, offer the first
> 4656 * page block on the list.
> 4657 *
> 4658 * Return 0 when a page block is found on the caller specified free list.
> 4659 */
> 4660 int report_unused_page_block(struct zone *zone, unsigned int order,
> 4661 unsigned int migratetype, struct page **page)
> 4662 {
> > 4663 struct zone *this_zone;
> 4664 struct list_head *this_list;
> 4665 int ret = 0;
> 4666 unsigned long flags;
> 4667
> 4668 /* Sanity check */
> 4669 if (zone == NULL || page == NULL || order >= MAX_ORDER ||
> 4670 migratetype >= MIGRATE_TYPES)
> 4671 return -EINVAL;
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH 1/1] s390/virtio: change maintainership
From: Michael S. Tsirkin @ 2017-05-05 20:37 UTC (permalink / raw)
To: Cornelia Huck; +Cc: linux-s390, kvm, virtualization
In-Reply-To: <20170505154452.1c04d328.cornelia.huck@de.ibm.com>
On Fri, May 05, 2017 at 03:44:52PM +0200, Cornelia Huck wrote:
> On Thu, 20 Apr 2017 09:51:29 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
> > On 03/02/2017 05:08 PM, Christian Borntraeger wrote:
> > > Halil is doing a lot more work in the virtio area on s390 than I
> > > do. Let's reflect the reality in the maintainers file.
> > >
> > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> > > Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > > ---
> > > MAINTAINERS | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index ca6f5f7..52780b8 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -13203,8 +13203,8 @@ F: include/uapi/linux/virtio_*.h
> > > F: drivers/crypto/virtio/
> > >
> > > VIRTIO DRIVERS FOR S390
> > > -M: Christian Borntraeger <borntraeger@de.ibm.com>
> > > M: Cornelia Huck <cornelia.huck@de.ibm.com>
> > > +M: Halil Pasic <pasic@linux.vnet.ibm.com>
> > > L: linux-s390@vger.kernel.org
> > > L: virtualization@lists.linux-foundation.org
> > > L: kvm@vger.kernel.org
> > >
> >
> > ping?
>
> ping^2
Will apply, thanks.
^ permalink raw reply
* Re: [PATCH 1/1] s390/virtio: change maintainership
From: Cornelia Huck @ 2017-05-05 13:44 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang; +Cc: linux-s390, kvm, virtualization
In-Reply-To: <00940a05-e752-a6bb-8ba3-a9e50732daf6@de.ibm.com>
On Thu, 20 Apr 2017 09:51:29 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 03/02/2017 05:08 PM, Christian Borntraeger wrote:
> > Halil is doing a lot more work in the virtio area on s390 than I
> > do. Let's reflect the reality in the maintainers file.
> >
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> > Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> > MAINTAINERS | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ca6f5f7..52780b8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13203,8 +13203,8 @@ F: include/uapi/linux/virtio_*.h
> > F: drivers/crypto/virtio/
> >
> > VIRTIO DRIVERS FOR S390
> > -M: Christian Borntraeger <borntraeger@de.ibm.com>
> > M: Cornelia Huck <cornelia.huck@de.ibm.com>
> > +M: Halil Pasic <pasic@linux.vnet.ibm.com>
> > L: linux-s390@vger.kernel.org
> > L: virtualization@lists.linux-foundation.org
> > L: kvm@vger.kernel.org
> >
>
> ping?
ping^2
^ permalink raw reply
* Re: [PATCH v10 4/6] mm: function to offer a page block on the free list
From: kbuild test robot @ 2017-05-05 0:21 UTC (permalink / raw)
To: Wei Wang
Cc: aarcange, virtio-dev, amit.shah, kvm, mst, linux-kernel,
liliang.opensource, dave.hansen, qemu-devel, virtualization,
linux-mm, kbuild-all, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <1493887815-6070-5-git-send-email-wei.w.wang@intel.com>
[-- Attachment #1: Type: text/plain, Size: 13834 bytes --]
Hi Wei,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.11 next-20170504]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Wei-Wang/Extend-virtio-balloon-for-fast-de-inflating-fast-live-migration/20170505-052958
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
arch/x86/include/asm/uaccess_32.h:1: warning: no structured comments found
>> mm/page_alloc.c:4663: warning: No description found for parameter 'zone'
>> mm/page_alloc.c:4663: warning: No description found for parameter 'order'
>> mm/page_alloc.c:4663: warning: No description found for parameter 'migratetype'
>> mm/page_alloc.c:4663: warning: No description found for parameter 'page'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
include/net/cfg80211.h:1738: warning: No description found for parameter 'report_results'
include/net/cfg80211.h:1738: warning: Excess struct/union/enum/typedef member 'results_wk' description in 'cfg80211_sched_scan_request'
vim +/zone +4663 mm/page_alloc.c
4647 * Heuristically get a page block in the system that is unused.
4648 * It is possible that pages from the page block are used immediately after
4649 * report_unused_page_block() returns. It is the caller's responsibility
4650 * to either detect or prevent the use of such pages.
4651 *
4652 * The free list to check: zone->free_area[order].free_list[migratetype].
4653 *
4654 * If the caller supplied page block (i.e. **page) is on the free list, offer
4655 * the next page block on the list to the caller. Otherwise, offer the first
4656 * page block on the list.
4657 *
4658 * Return 0 when a page block is found on the caller specified free list.
4659 */
4660 int report_unused_page_block(struct zone *zone, unsigned int order,
4661 unsigned int migratetype, struct page **page)
4662 {
> 4663 struct zone *this_zone;
4664 struct list_head *this_list;
4665 int ret = 0;
4666 unsigned long flags;
4667
4668 /* Sanity check */
4669 if (zone == NULL || page == NULL || order >= MAX_ORDER ||
4670 migratetype >= MIGRATE_TYPES)
4671 return -EINVAL;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6564 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH v10 6/6] virtio-balloon: VIRTIO_BALLOON_F_MISC_VQ
From: Wei Wang @ 2017-05-04 8:50 UTC (permalink / raw)
To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1493887815-6070-1-git-send-email-wei.w.wang@intel.com>
Add a new vq, miscq, to handle miscellaneous requests between the device
and the driver. Only one request is handled in-flight each time.
This patch implements the VIRTIO_BALLOON_MISCQ_CMD_REPORT_UNUSED_PAGES
request sent from the device. Upon receiving the request from the
miscq, the driver offers to the device the guest unused pages.
Tests have shown that skipping the transfer of unused pages of a 32G
idle guest can get the live migration time reduced to 1/8.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Liang Li <liang.z.li@intel.com>
---
drivers/virtio/virtio_balloon.c | 299 +++++++++++++++++++++++++++++++-----
include/uapi/linux/virtio_balloon.h | 12 ++
2 files changed, 274 insertions(+), 37 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index df16912..4dcee2c 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -57,6 +57,10 @@
/* The maximum number of page_bmap that can be allocated. */
#define VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM 32
+/* Types of pages to chunk */
+#define PAGE_CHUNK_TYPE_BALLOON 0 /* Chunk of inflate/deflate pages */
+#define PAGE_CHUNK_TYPE_UNUSED 1 /* Chunk of unused pages */
+
static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
module_param(oom_pages, int, S_IRUSR | S_IWUSR);
MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
@@ -67,16 +71,17 @@ static struct vfsmount *balloon_mnt;
/* Maximum number of page chunks */
#define VIRTIO_BALLOON_MAX_PAGE_CHUNKS ((8 * PAGE_SIZE - \
- sizeof(struct virtio_balloon_page_chunk)) / \
- sizeof(struct virtio_balloon_page_chunk_entry))
+ sizeof(struct virtio_balloon_miscq_msg)) / \
+ sizeof(struct virtio_balloon_page_chunk_entry))
struct virtio_balloon {
struct virtio_device *vdev;
- struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
+ struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *miscq;
/* The balloon servicing is delegated to a freezable workqueue. */
struct work_struct update_balloon_stats_work;
struct work_struct update_balloon_size_work;
+ struct work_struct miscq_handle_work;
/* Prevent updating balloon when it is being canceled. */
spinlock_t stop_update_lock;
@@ -98,6 +103,9 @@ struct virtio_balloon {
/* Synchronize access/update to this struct virtio_balloon elements */
struct mutex balloon_lock;
+ /* Miscq msg buffer for the REPORT_UNUSED_PAGES cmd */
+ struct virtio_balloon_miscq_msg *miscq_msg_rup;
+
/* Buffer for chunks of ballooned pages. */
struct virtio_balloon_page_chunk *balloon_page_chunk;
@@ -200,38 +208,85 @@ static void clear_page_bmap(struct virtio_balloon *vb,
memset(vb->page_bmap[i], 0, VIRTIO_BALLOON_PAGE_BMAP_SIZE);
}
-static void send_page_chunks(struct virtio_balloon *vb, struct virtqueue *vq)
+static void send_page_chunks(struct virtio_balloon *vb, struct virtqueue *vq,
+ int type, bool busy_wait)
{
struct scatterlist sg;
struct virtio_balloon_page_chunk *chunk;
- unsigned int len;
+ void *msg_buf;
+ unsigned int msg_len;
+ uint64_t chunk_num = 0;
+
+ switch (type) {
+ case PAGE_CHUNK_TYPE_BALLOON:
+ chunk = vb->balloon_page_chunk;
+ chunk_num = le64_to_cpu(chunk->chunk_num);
+ msg_buf = vb->balloon_page_chunk;
+ msg_len = sizeof(struct virtio_balloon_page_chunk) +
+ sizeof(struct virtio_balloon_page_chunk_entry) *
+ chunk_num;
+ break;
+ case PAGE_CHUNK_TYPE_UNUSED:
+ chunk = &vb->miscq_msg_rup->payload.chunk;
+ chunk_num = le64_to_cpu(chunk->chunk_num);
+ msg_buf = vb->miscq_msg_rup;
+ msg_len = sizeof(struct virtio_balloon_miscq_msg) +
+ sizeof(struct virtio_balloon_page_chunk_entry) *
+ chunk_num;
+ break;
+ default:
+ dev_warn(&vb->vdev->dev, "%s: chunk %d of unknown pages\n",
+ __func__, type);
+ return;
+ }
- chunk = vb->balloon_page_chunk;
- len = sizeof(__le64) +
- le64_to_cpu(chunk->chunk_num) *
- sizeof(struct virtio_balloon_page_chunk_entry);
- sg_init_one(&sg, chunk, len);
+ sg_init_one(&sg, msg_buf, msg_len);
if (!virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL)) {
virtqueue_kick(vq);
- wait_event(vb->acked, virtqueue_get_buf(vq, &len));
+ if (busy_wait)
+ while (!virtqueue_get_buf(vq, &msg_len) &&
+ !virtqueue_is_broken(vq))
+ cpu_relax();
+ else
+ wait_event(vb->acked, virtqueue_get_buf(vq, &msg_len));
+ /*
+ * Now, the chunks have been delivered to the host.
+ * Reset the filed in the structure that records the number of
+ * added chunks, so that new added chunks can be re-counted.
+ */
chunk->chunk_num = 0;
}
}
/* Add a chunk entry to the buffer. */
static void add_one_chunk(struct virtio_balloon *vb, struct virtqueue *vq,
- u64 base, u64 size)
+ int type, u64 base, u64 size)
{
- struct virtio_balloon_page_chunk *chunk = vb->balloon_page_chunk;
+ struct virtio_balloon_page_chunk *chunk;
struct virtio_balloon_page_chunk_entry *entry;
- uint64_t chunk_num = le64_to_cpu(chunk->chunk_num);
-
+ uint64_t chunk_num;
+
+ switch (type) {
+ case PAGE_CHUNK_TYPE_BALLOON:
+ chunk = vb->balloon_page_chunk;
+ chunk_num = le64_to_cpu(vb->balloon_page_chunk->chunk_num);
+ break;
+ case PAGE_CHUNK_TYPE_UNUSED:
+ chunk = &vb->miscq_msg_rup->payload.chunk;
+ chunk_num =
+ le64_to_cpu(vb->miscq_msg_rup->payload.chunk.chunk_num);
+ break;
+ default:
+ dev_warn(&vb->vdev->dev, "%s: chunk %d of unknown pages\n",
+ __func__, type);
+ return;
+ }
entry = &chunk->entry[chunk_num];
entry->base = cpu_to_le64(base << VIRTIO_BALLOON_CHUNK_BASE_SHIFT);
entry->size = cpu_to_le64(size << VIRTIO_BALLOON_CHUNK_SIZE_SHIFT);
chunk->chunk_num = cpu_to_le64(++chunk_num);
if (chunk_num == VIRTIO_BALLOON_MAX_PAGE_CHUNKS)
- send_page_chunks(vb, vq);
+ send_page_chunks(vb, vq, type, 0);
}
static void convert_bmap_to_chunks(struct virtio_balloon *vb,
@@ -259,8 +314,8 @@ static void convert_bmap_to_chunks(struct virtio_balloon *vb,
chunk_size = (next_zero - next_one) *
VIRTIO_BALLOON_PAGES_PER_PAGE;
if (chunk_size) {
- add_one_chunk(vb, vq, pfn_start + next_one,
- chunk_size);
+ add_one_chunk(vb, vq, PAGE_CHUNK_TYPE_BALLOON,
+ pfn_start + next_one, chunk_size);
pos += next_zero + 1;
}
}
@@ -305,7 +360,7 @@ static void tell_host_from_page_bmap(struct virtio_balloon *vb,
pfn_num);
}
if (le64_to_cpu(vb->balloon_page_chunk->chunk_num) > 0)
- send_page_chunks(vb, vq);
+ send_page_chunks(vb, vq, PAGE_CHUNK_TYPE_BALLOON, 0);
}
static void set_page_pfns(struct virtio_balloon *vb,
@@ -679,43 +734,186 @@ static void update_balloon_size_func(struct work_struct *work)
queue_work(system_freezable_wq, work);
}
+/* Add a message buffer for the host to fill in a request */
+static void miscq_msg_inbuf_add(struct virtio_balloon *vb,
+ struct virtio_balloon_miscq_msg *req_buf)
+{
+ struct scatterlist sg_in;
+
+ sg_init_one(&sg_in, req_buf, sizeof(struct virtio_balloon_miscq_msg));
+ if (virtqueue_add_inbuf(vb->miscq, &sg_in, 1, req_buf, GFP_KERNEL)
+ < 0) {
+ __virtio_clear_bit(vb->vdev,
+ VIRTIO_BALLOON_F_MISC_VQ);
+ dev_warn(&vb->vdev->dev, "%s: add miscq msg buf err\n",
+ __func__);
+ return;
+ }
+ virtqueue_kick(vb->miscq);
+}
+
+static void miscq_report_unused_pages(struct virtio_balloon *vb)
+{
+ struct virtio_balloon_miscq_msg *msg = vb->miscq_msg_rup;
+ struct virtqueue *vq = vb->miscq;
+ int ret = 0;
+ unsigned int order = 0, migratetype = 0;
+ struct zone *zone = NULL;
+ struct page *page = NULL;
+ u64 pfn;
+
+ msg->cmd = cpu_to_le32(VIRTIO_BALLOON_MISCQ_CMD_REPORT_UNUSED_PAGES);
+ msg->flags = 0;
+
+ for_each_populated_zone(zone) {
+ for (order = MAX_ORDER - 1; order > 0; order--) {
+ for (migratetype = 0; migratetype < MIGRATE_TYPES;
+ migratetype++) {
+ do {
+ ret = report_unused_page_block(zone,
+ order, migratetype, &page);
+ if (!ret) {
+ pfn = (u64)page_to_pfn(page);
+ add_one_chunk(vb, vq,
+ PAGE_CHUNK_TYPE_UNUSED,
+ pfn,
+ (u64)(1 << order) *
+ VIRTIO_BALLOON_PAGES_PER_PAGE);
+ }
+ } while (!ret);
+ }
+ }
+ }
+ /* Set the cmd completion flag */
+ msg->flags |= cpu_to_le32(VIRTIO_BALLOON_MISCQ_F_COMPLETION);
+ send_page_chunks(vb, vq, PAGE_CHUNK_TYPE_UNUSED, true);
+}
+
+static void miscq_handle_func(struct work_struct *work)
+{
+ struct virtio_balloon *vb;
+ struct virtio_balloon_miscq_msg *msg;
+ unsigned int len;
+
+ vb = container_of(work, struct virtio_balloon,
+ miscq_handle_work);
+ msg = virtqueue_get_buf(vb->miscq, &len);
+ if (!msg || len != sizeof(struct virtio_balloon_miscq_msg)) {
+ dev_warn(&vb->vdev->dev, "%s: invalid miscq msg len\n",
+ __func__);
+ miscq_msg_inbuf_add(vb, vb->miscq_msg_rup);
+ return;
+ }
+ switch (msg->cmd) {
+ case VIRTIO_BALLOON_MISCQ_CMD_REPORT_UNUSED_PAGES:
+ miscq_report_unused_pages(vb);
+ break;
+ default:
+ dev_warn(&vb->vdev->dev, "%s: miscq cmd %d not supported\n",
+ __func__, msg->cmd);
+ }
+ miscq_msg_inbuf_add(vb, vb->miscq_msg_rup);
+}
+
+static void miscq_request(struct virtqueue *vq)
+{
+ struct virtio_balloon *vb = vq->vdev->priv;
+
+ queue_work(system_freezable_wq, &vb->miscq_handle_work);
+}
+
static int init_vqs(struct virtio_balloon *vb)
{
- struct virtqueue *vqs[3];
- vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request };
- static const char * const names[] = { "inflate", "deflate", "stats" };
- int err, nvqs;
+ struct virtqueue **vqs;
+ vq_callback_t **callbacks;
+ const char **names;
+ int err = -ENOMEM;
+ int i, nvqs;
+
+ /* Inflateq and deflateq are used unconditionally */
+ nvqs = 2;
+
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ))
+ nvqs++;
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_MISC_VQ))
+ nvqs++;
+
+ /* Allocate space for find_vqs parameters */
+ vqs = kcalloc(nvqs, sizeof(*vqs), GFP_KERNEL);
+ if (!vqs)
+ goto err_vq;
+ callbacks = kmalloc_array(nvqs, sizeof(*callbacks), GFP_KERNEL);
+ if (!callbacks)
+ goto err_callback;
+ names = kmalloc_array(nvqs, sizeof(*names), GFP_KERNEL);
+ if (!names)
+ goto err_names;
+
+ callbacks[0] = balloon_ack;
+ names[0] = "inflate";
+ callbacks[1] = balloon_ack;
+ names[1] = "deflate";
+
+ i = 2;
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
+ callbacks[i] = stats_request;
+ names[i] = "stats";
+ i++;
+ }
- /*
- * We expect two virtqueues: inflate and deflate, and
- * optionally stat.
- */
- nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
- err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names,
- NULL);
+ if (virtio_has_feature(vb->vdev,
+ VIRTIO_BALLOON_F_MISC_VQ)) {
+ callbacks[i] = miscq_request;
+ names[i] = "miscq";
+ }
+
+ err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks,
+ names, NULL);
if (err)
- return err;
+ goto err_find;
vb->inflate_vq = vqs[0];
vb->deflate_vq = vqs[1];
+ i = 2;
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
struct scatterlist sg;
- unsigned int num_stats;
- vb->stats_vq = vqs[2];
+ vb->stats_vq = vqs[i++];
/*
* Prime this virtqueue with one buffer so the hypervisor can
* use it to signal us later (it can't be broken yet!).
*/
- num_stats = update_balloon_stats(vb);
-
- sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
+ sg_init_one(&sg, vb->stats, sizeof(vb->stats));
if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL)
< 0)
BUG();
virtqueue_kick(vb->stats_vq);
}
+
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_MISC_VQ)) {
+ vb->miscq = vqs[i];
+ /*
+ * Add the msg buf for the REPORT_UNUSED_PAGES request.
+ * The request is handled one in-flight each time. So, just
+ * use the response buffer, msicq_msg_rup, for the host to
+ * fill in a request.
+ */
+ miscq_msg_inbuf_add(vb, vb->miscq_msg_rup);
+ }
+
+ kfree(names);
+ kfree(callbacks);
+ kfree(vqs);
return 0;
+
+err_find:
+ kfree(names);
+err_names:
+ kfree(callbacks);
+err_callback:
+ kfree(vqs);
+err_vq:
+ return err;
}
#ifdef CONFIG_BALLOON_COMPACTION
@@ -723,7 +921,7 @@ static int init_vqs(struct virtio_balloon *vb)
static void tell_host_one_page(struct virtio_balloon *vb,
struct virtqueue *vq, struct page *page)
{
- add_one_chunk(vb, vq, page_to_pfn(page),
+ add_one_chunk(vb, vq, PAGE_CHUNK_TYPE_BALLOON, page_to_pfn(page),
VIRTIO_BALLOON_PAGES_PER_PAGE);
}
@@ -857,6 +1055,22 @@ static int balloon_page_chunk_init(struct virtio_balloon *vb)
return -ENOMEM;
}
+static int miscq_init(struct virtio_balloon *vb)
+{
+ vb->miscq_msg_rup = kmalloc(sizeof(struct virtio_balloon_miscq_msg) +
+ sizeof(struct virtio_balloon_page_chunk_entry) *
+ VIRTIO_BALLOON_MAX_PAGE_CHUNKS, GFP_KERNEL);
+ if (!vb->miscq_msg_rup) {
+ __virtio_clear_bit(vb->vdev, VIRTIO_BALLOON_F_MISC_VQ);
+ dev_warn(&vb->vdev->dev, "%s: failed\n", __func__);
+ return -ENOMEM;
+ }
+
+ INIT_WORK(&vb->miscq_handle_work, miscq_handle_func);
+
+ return 0;
+}
+
static int virtballoon_validate(struct virtio_device *vdev)
{
struct virtio_balloon *vb = NULL;
@@ -874,7 +1088,16 @@ static int virtballoon_validate(struct virtio_device *vdev)
goto err_page_chunk;
}
+ if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_MISC_VQ)) {
+ err = miscq_init(vb);
+ if (err < 0)
+ goto err_miscq_rup;
+ }
+
return 0;
+err_miscq_rup:
+ free_page_bmap(vb);
+ kfree(vb->balloon_page_chunk);
err_page_chunk:
kfree(vb);
err_vb:
@@ -971,6 +1194,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
spin_unlock_irq(&vb->stop_update_lock);
cancel_work_sync(&vb->update_balloon_size_work);
cancel_work_sync(&vb->update_balloon_stats_work);
+ cancel_work_sync(&vb->miscq_handle_work);
remove_common(vb);
free_page_bmap(vb);
@@ -1020,6 +1244,7 @@ static unsigned int features[] = {
VIRTIO_BALLOON_F_STATS_VQ,
VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
VIRTIO_BALLOON_F_PAGE_CHUNKS,
+ VIRTIO_BALLOON_F_MISC_VQ,
};
static struct virtio_driver virtio_balloon_driver = {
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index d532ed16..ea83b74 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -35,6 +35,7 @@
#define VIRTIO_BALLOON_F_STATS_VQ 1 /* Memory Stats virtqueue */
#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
#define VIRTIO_BALLOON_F_PAGE_CHUNKS 3 /* Inflate/Deflate pages in chunks */
+#define VIRTIO_BALLOON_F_MISC_VQ 4 /* Virtqueue for misc. requests */
/* Size of a PFN in the balloon interface. */
#define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -96,4 +97,15 @@ struct virtio_balloon_page_chunk {
struct virtio_balloon_page_chunk_entry entry[];
};
+struct virtio_balloon_miscq_msg {
+#define VIRTIO_BALLOON_MISCQ_CMD_REPORT_UNUSED_PAGES 0
+ __le32 cmd;
+/* Flag to indicate the completion of handling a command */
+#define VIRTIO_BALLOON_MISCQ_F_COMPLETION 1
+ __le32 flags;
+ union {
+ struct virtio_balloon_page_chunk chunk;
+ } payload;
+};
+
#endif /* _LINUX_VIRTIO_BALLOON_H */
--
2.7.4
^ permalink raw reply related
* [PATCH v10 5/6] mm: export symbol of next_zone and first_online_pgdat
From: Wei Wang @ 2017-05-04 8:50 UTC (permalink / raw)
To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1493887815-6070-1-git-send-email-wei.w.wang@intel.com>
This patch enables for_each_zone()/for_each_populated_zone() to be
invoked by a kernel module.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
mm/mmzone.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/mmzone.c b/mm/mmzone.c
index a51c0a6..08a2a3a 100644
--- a/mm/mmzone.c
+++ b/mm/mmzone.c
@@ -13,6 +13,7 @@ struct pglist_data *first_online_pgdat(void)
{
return NODE_DATA(first_online_node);
}
+EXPORT_SYMBOL_GPL(first_online_pgdat);
struct pglist_data *next_online_pgdat(struct pglist_data *pgdat)
{
@@ -41,6 +42,7 @@ struct zone *next_zone(struct zone *zone)
}
return zone;
}
+EXPORT_SYMBOL_GPL(next_zone);
static inline int zref_in_nodemask(struct zoneref *zref, nodemask_t *nodes)
{
--
2.7.4
^ permalink raw reply related
* [PATCH v10 4/6] mm: function to offer a page block on the free list
From: Wei Wang @ 2017-05-04 8:50 UTC (permalink / raw)
To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1493887815-6070-1-git-send-email-wei.w.wang@intel.com>
Add a function to find a page block on the free list specified by the
caller. Pages from the page block may be used immediately after the
function returns. The caller is responsible for detecting or preventing
the use of such pages.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Liang Li <liang.z.li@intel.com>
---
include/linux/mm.h | 5 +++
mm/page_alloc.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5d22e69..82361a6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1841,6 +1841,11 @@ extern void free_area_init_node(int nid, unsigned long * zones_size,
unsigned long zone_start_pfn, unsigned long *zholes_size);
extern void free_initmem(void);
+#if IS_ENABLED(CONFIG_VIRTIO_BALLOON)
+extern int report_unused_page_block(struct zone *zone, unsigned int order,
+ unsigned int migratetype,
+ struct page **page);
+#endif
/*
* Free reserved pages within range [PAGE_ALIGN(start), end & PAGE_MASK)
* into the buddy system. The freed pages will be poisoned with pattern
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2c25de4..e554ab8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4615,6 +4615,97 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
show_swap_cache_info();
}
+#if IS_ENABLED(CONFIG_VIRTIO_BALLOON)
+
+/**
+ * Heuristically get a page block in the system that is unused.
+ * It is possible that pages from the page block are used immediately after
+ * report_unused_page_block() returns. It is the caller's responsibility
+ * to either detect or prevent the use of such pages.
+ *
+ * The free list to check: zone->free_area[order].free_list[migratetype].
+ *
+ * If the caller supplied page block (i.e. **page) is on the free list, offer
+ * the next page block on the list to the caller. Otherwise, offer the first
+ * page block on the list.
+ *
+ * Return 0 when a page block is found on the caller specified free list.
+ */
+int report_unused_page_block(struct zone *zone, unsigned int order,
+ unsigned int migratetype, struct page **page)
+{
+ struct zone *this_zone;
+ struct list_head *this_list;
+ int ret = 0;
+ unsigned long flags;
+
+ /* Sanity check */
+ if (zone == NULL || page == NULL || order >= MAX_ORDER ||
+ migratetype >= MIGRATE_TYPES)
+ return -EINVAL;
+
+ /* Zone validity check */
+ for_each_populated_zone(this_zone) {
+ if (zone == this_zone)
+ break;
+ }
+
+ /* Got a non-existent zone from the caller? */
+ if (zone != this_zone)
+ return -EINVAL;
+
+ spin_lock_irqsave(&this_zone->lock, flags);
+
+ this_list = &zone->free_area[order].free_list[migratetype];
+ if (list_empty(this_list)) {
+ *page = NULL;
+ ret = 1;
+ goto out;
+ }
+
+ /* The caller is asking for the first free page block on the list */
+ if ((*page) == NULL) {
+ *page = list_first_entry(this_list, struct page, lru);
+ ret = 0;
+ goto out;
+ }
+
+ /**
+ * The page block passed from the caller is not on this free list
+ * anymore (e.g. a 1MB free page block has been split). In this case,
+ * offer the first page block on the free list that the caller is
+ * asking for.
+ */
+ if (PageBuddy(*page) && order != page_order(*page)) {
+ *page = list_first_entry(this_list, struct page, lru);
+ ret = 0;
+ goto out;
+ }
+
+ /**
+ * The page block passed from the caller has been the last page block
+ * on the list.
+ */
+ if ((*page)->lru.next == this_list) {
+ *page = NULL;
+ ret = 1;
+ goto out;
+ }
+
+ /**
+ * Finally, fall into the regular case: the page block passed from the
+ * caller is still on the free list. Offer the next one.
+ */
+ *page = list_next_entry((*page), lru);
+ ret = 0;
+out:
+ spin_unlock_irqrestore(&this_zone->lock, flags);
+ return ret;
+}
+EXPORT_SYMBOL(report_unused_page_block);
+
+#endif
+
static void zoneref_set_zone(struct zone *zone, struct zoneref *zoneref)
{
zoneref->zone = zone;
--
2.7.4
^ permalink raw reply related
* [PATCH v10 3/6] virtio-balloon: VIRTIO_BALLOON_F_PAGE_CHUNKS
From: Wei Wang @ 2017-05-04 8:50 UTC (permalink / raw)
To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1493887815-6070-1-git-send-email-wei.w.wang@intel.com>
Add a new feature, VIRTIO_BALLOON_F_PAGE_CHUNKS, which enables
the transfer of the ballooned (i.e. inflated/deflated) pages in
chunks to the host.
The implementation of the previous virtio-balloon is not very
efficient, because the ballooned pages are transferred to the
host one by one. Here is the breakdown of the time in percentage
spent on each step of the balloon inflating process (inflating
7GB of an 8GB idle guest).
1) allocating pages (6.5%)
2) sending PFNs to host (68.3%)
3) address translation (6.1%)
4) madvise (19%)
It takes about 4126ms for the inflating process to complete.
The above profiling shows that the bottlenecks are stage 2)
and stage 4).
This patch optimizes step 2) by transferring pages to the host in
chunks. A chunk consists of guest physically continuous pages.
When the pages are packed into a chunk, they are converted into
balloon page size (4KB) pages. A chunk is offered to the host
via a base PFN (i.e. the start PFN of those physically continuous
pages) and the size (i.e. the total number of the 4KB balloon size
pages). A chunk is formatted as below:
--------------------------------------------------------
| Base (52 bit) | Rsvd (12 bit) |
--------------------------------------------------------
--------------------------------------------------------
| Size (52 bit) | Rsvd (12 bit) |
--------------------------------------------------------
By doing so, step 4) can also be optimized by doing address
translation and madvise() in chunks rather than page by page.
With this new feature, the above ballooning process takes ~590ms
resulting in an improvement of ~85%.
TODO: optimize stage 1) by allocating/freeing a chunk of pages
instead of a single page each time.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Liang Li <liang.z.li@intel.com>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_balloon.c | 407 +++++++++++++++++++++++++++++++++---
include/uapi/linux/virtio_balloon.h | 14 ++
2 files changed, 396 insertions(+), 25 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index ecb64e9..df16912 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -43,6 +43,20 @@
#define OOM_VBALLOON_DEFAULT_PAGES 256
#define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
+/* The size of one page_bmap used to record inflated/deflated pages. */
+#define VIRTIO_BALLOON_PAGE_BMAP_SIZE (8 * PAGE_SIZE)
+/*
+ * Callulates how many pfns can a page_bmap record. A bit corresponds to a
+ * page of PAGE_SIZE.
+ */
+#define VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP \
+ (VIRTIO_BALLOON_PAGE_BMAP_SIZE * BITS_PER_BYTE)
+
+/* The number of page_bmap to allocate by default. */
+#define VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM 1
+/* The maximum number of page_bmap that can be allocated. */
+#define VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM 32
+
static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
module_param(oom_pages, int, S_IRUSR | S_IWUSR);
MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
@@ -51,6 +65,11 @@ MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
static struct vfsmount *balloon_mnt;
#endif
+/* Maximum number of page chunks */
+#define VIRTIO_BALLOON_MAX_PAGE_CHUNKS ((8 * PAGE_SIZE - \
+ sizeof(struct virtio_balloon_page_chunk)) / \
+ sizeof(struct virtio_balloon_page_chunk_entry))
+
struct virtio_balloon {
struct virtio_device *vdev;
struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
@@ -79,6 +98,12 @@ struct virtio_balloon {
/* Synchronize access/update to this struct virtio_balloon elements */
struct mutex balloon_lock;
+ /* Buffer for chunks of ballooned pages. */
+ struct virtio_balloon_page_chunk *balloon_page_chunk;
+
+ /* Bitmap used to record pages. */
+ unsigned long *page_bmap[VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM];
+
/* The array of pfns we tell the Host about. */
unsigned int num_pfns;
__virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
@@ -111,6 +136,136 @@ static void balloon_ack(struct virtqueue *vq)
wake_up(&vb->acked);
}
+/* Update pfn_max and pfn_min according to the pfn of page */
+static inline void update_pfn_range(struct virtio_balloon *vb,
+ struct page *page,
+ unsigned long *pfn_min,
+ unsigned long *pfn_max)
+{
+ unsigned long pfn = page_to_pfn(page);
+
+ *pfn_min = min(pfn, *pfn_min);
+ *pfn_max = max(pfn, *pfn_max);
+}
+
+static unsigned int extend_page_bmap_size(struct virtio_balloon *vb,
+ unsigned long pfn_num)
+{
+ unsigned int i, bmap_num, allocated_bmap_num;
+ unsigned long bmap_len;
+
+ allocated_bmap_num = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM;
+ bmap_len = ALIGN(pfn_num, BITS_PER_LONG) / BITS_PER_BYTE;
+ bmap_len = roundup(bmap_len, VIRTIO_BALLOON_PAGE_BMAP_SIZE);
+ /*
+ * VIRTIO_BALLOON_PAGE_BMAP_SIZE is the size of one page_bmap, so
+ * divide it to calculate how many page_bmap that we need.
+ */
+ bmap_num = (unsigned int)(bmap_len / VIRTIO_BALLOON_PAGE_BMAP_SIZE);
+ /* The number of page_bmap to allocate should not exceed the max */
+ bmap_num = min_t(unsigned int, VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM,
+ bmap_num);
+
+ for (i = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i < bmap_num; i++) {
+ vb->page_bmap[i] = kmalloc(VIRTIO_BALLOON_PAGE_BMAP_SIZE,
+ GFP_KERNEL);
+ if (vb->page_bmap[i])
+ allocated_bmap_num++;
+ else
+ break;
+ }
+
+ return allocated_bmap_num;
+}
+
+static void free_extended_page_bmap(struct virtio_balloon *vb,
+ unsigned int page_bmap_num)
+{
+ unsigned int i;
+
+ for (i = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i < page_bmap_num;
+ i++) {
+ kfree(vb->page_bmap[i]);
+ vb->page_bmap[i] = NULL;
+ page_bmap_num--;
+ }
+}
+
+static void clear_page_bmap(struct virtio_balloon *vb,
+ unsigned int page_bmap_num)
+{
+ int i;
+
+ for (i = 0; i < page_bmap_num; i++)
+ memset(vb->page_bmap[i], 0, VIRTIO_BALLOON_PAGE_BMAP_SIZE);
+}
+
+static void send_page_chunks(struct virtio_balloon *vb, struct virtqueue *vq)
+{
+ struct scatterlist sg;
+ struct virtio_balloon_page_chunk *chunk;
+ unsigned int len;
+
+ chunk = vb->balloon_page_chunk;
+ len = sizeof(__le64) +
+ le64_to_cpu(chunk->chunk_num) *
+ sizeof(struct virtio_balloon_page_chunk_entry);
+ sg_init_one(&sg, chunk, len);
+ if (!virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL)) {
+ virtqueue_kick(vq);
+ wait_event(vb->acked, virtqueue_get_buf(vq, &len));
+ chunk->chunk_num = 0;
+ }
+}
+
+/* Add a chunk entry to the buffer. */
+static void add_one_chunk(struct virtio_balloon *vb, struct virtqueue *vq,
+ u64 base, u64 size)
+{
+ struct virtio_balloon_page_chunk *chunk = vb->balloon_page_chunk;
+ struct virtio_balloon_page_chunk_entry *entry;
+ uint64_t chunk_num = le64_to_cpu(chunk->chunk_num);
+
+ entry = &chunk->entry[chunk_num];
+ entry->base = cpu_to_le64(base << VIRTIO_BALLOON_CHUNK_BASE_SHIFT);
+ entry->size = cpu_to_le64(size << VIRTIO_BALLOON_CHUNK_SIZE_SHIFT);
+ chunk->chunk_num = cpu_to_le64(++chunk_num);
+ if (chunk_num == VIRTIO_BALLOON_MAX_PAGE_CHUNKS)
+ send_page_chunks(vb, vq);
+}
+
+static void convert_bmap_to_chunks(struct virtio_balloon *vb,
+ struct virtqueue *vq,
+ unsigned long *bmap,
+ unsigned long pfn_start,
+ unsigned long size)
+{
+ unsigned long next_one, next_zero, chunk_size, pos = 0;
+
+ while (pos < size) {
+ next_one = find_next_bit(bmap, size, pos);
+ /*
+ * No "1" bit found, which means that there is no pfn
+ * recorded in the rest of this bmap.
+ */
+ if (next_one == size)
+ break;
+ next_zero = find_next_zero_bit(bmap, size, next_one + 1);
+ /*
+ * A bit in page_bmap corresponds to a page of PAGE_SIZE.
+ * Convert it to be pages of 4KB balloon page size when
+ * adding it to a chunk.
+ */
+ chunk_size = (next_zero - next_one) *
+ VIRTIO_BALLOON_PAGES_PER_PAGE;
+ if (chunk_size) {
+ add_one_chunk(vb, vq, pfn_start + next_one,
+ chunk_size);
+ pos += next_zero + 1;
+ }
+ }
+}
+
static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
{
struct scatterlist sg;
@@ -124,7 +279,33 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
/* When host has read buffer, this completes via balloon_ack */
wait_event(vb->acked, virtqueue_get_buf(vq, &len));
+}
+
+static void tell_host_from_page_bmap(struct virtio_balloon *vb,
+ struct virtqueue *vq,
+ unsigned long pfn_start,
+ unsigned long pfn_end,
+ unsigned int page_bmap_num)
+{
+ unsigned long i, pfn_num;
+ for (i = 0; i < page_bmap_num; i++) {
+ /*
+ * For the last page_bmap, only the remaining number of pfns
+ * need to be searched rather than the entire page_bmap.
+ */
+ if (i + 1 == page_bmap_num)
+ pfn_num = (pfn_end - pfn_start) %
+ VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
+ else
+ pfn_num = VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
+
+ convert_bmap_to_chunks(vb, vq, vb->page_bmap[i], pfn_start +
+ i * VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP,
+ pfn_num);
+ }
+ if (le64_to_cpu(vb->balloon_page_chunk->chunk_num) > 0)
+ send_page_chunks(vb, vq);
}
static void set_page_pfns(struct virtio_balloon *vb,
@@ -141,13 +322,88 @@ static void set_page_pfns(struct virtio_balloon *vb,
page_to_balloon_pfn(page) + i);
}
+/*
+ * Send ballooned pages in chunks to host.
+ * The ballooned pages are recorded in page bitmaps. Each bit in a bitmap
+ * corresponds to a page of PAGE_SIZE. The page bitmaps are searched for
+ * continuous "1" bits, which correspond to continuous pages, to chunk.
+ * When packing those continuous pages into chunks, pages are converted into
+ * 4KB balloon pages.
+ *
+ * pfn_max and pfn_min form the range of pfns that need to use page bitmaps to
+ * record. If the range is too large to be recorded into the allocated page
+ * bitmaps, the page bitmaps are used multiple times to record the entire
+ * range of pfns.
+ */
+static void tell_host_page_chunks(struct virtio_balloon *vb,
+ struct list_head *pages,
+ struct virtqueue *vq,
+ unsigned long pfn_max,
+ unsigned long pfn_min)
+{
+ /*
+ * The pfn_start and pfn_end form the range of pfns that the allocated
+ * page_bmap can record in each round.
+ */
+ unsigned long pfn_start, pfn_end;
+ /* Total number of allocated page_bmap */
+ unsigned int page_bmap_num;
+ struct page *page;
+ bool found;
+
+ /*
+ * In the case that one page_bmap is not sufficient to record the pfn
+ * range, page_bmap will be extended by allocating more numbers of
+ * page_bmap.
+ */
+ page_bmap_num = extend_page_bmap_size(vb, pfn_max - pfn_min + 1);
+
+ /* Start from the beginning of the whole pfn range */
+ pfn_start = pfn_min;
+ while (pfn_start < pfn_max) {
+ pfn_end = pfn_start +
+ VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP * page_bmap_num;
+ pfn_end = pfn_end < pfn_max ? pfn_end : pfn_max;
+ clear_page_bmap(vb, page_bmap_num);
+ found = false;
+
+ list_for_each_entry(page, pages, lru) {
+ unsigned long bmap_idx, bmap_pos, this_pfn;
+
+ this_pfn = page_to_pfn(page);
+ if (this_pfn < pfn_start || this_pfn > pfn_end)
+ continue;
+ bmap_idx = (this_pfn - pfn_start) /
+ VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
+ bmap_pos = (this_pfn - pfn_start) %
+ VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
+ set_bit(bmap_pos, vb->page_bmap[bmap_idx]);
+
+ found = true;
+ }
+ if (found)
+ tell_host_from_page_bmap(vb, vq, pfn_start, pfn_end,
+ page_bmap_num);
+ /*
+ * Start the next round when pfn_start and pfn_end couldn't
+ * cover the whole pfn range given by pfn_max and pfn_min.
+ */
+ pfn_start = pfn_end;
+ }
+ free_extended_page_bmap(vb, page_bmap_num);
+}
+
static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
{
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
unsigned num_allocated_pages;
+ bool chunking = virtio_has_feature(vb->vdev,
+ VIRTIO_BALLOON_F_PAGE_CHUNKS);
+ unsigned long pfn_max = 0, pfn_min = ULONG_MAX;
/* We can only do one array worth at a time. */
- num = min(num, ARRAY_SIZE(vb->pfns));
+ if (!chunking)
+ num = min(num, ARRAY_SIZE(vb->pfns));
mutex_lock(&vb->balloon_lock);
for (vb->num_pfns = 0; vb->num_pfns < num;
@@ -162,7 +418,10 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
msleep(200);
break;
}
- set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+ if (chunking)
+ update_pfn_range(vb, page, &pfn_max, &pfn_min);
+ else
+ set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
if (!virtio_has_feature(vb->vdev,
VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
@@ -171,8 +430,14 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
num_allocated_pages = vb->num_pfns;
/* Did we get any? */
- if (vb->num_pfns != 0)
- tell_host(vb, vb->inflate_vq);
+ if (vb->num_pfns != 0) {
+ if (chunking)
+ tell_host_page_chunks(vb, &vb_dev_info->pages,
+ vb->inflate_vq,
+ pfn_max, pfn_min);
+ else
+ tell_host(vb, vb->inflate_vq);
+ }
mutex_unlock(&vb->balloon_lock);
return num_allocated_pages;
@@ -198,9 +463,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
struct page *page;
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
LIST_HEAD(pages);
+ bool chunking = virtio_has_feature(vb->vdev,
+ VIRTIO_BALLOON_F_PAGE_CHUNKS);
+ unsigned long pfn_max = 0, pfn_min = ULONG_MAX;
- /* We can only do one array worth at a time. */
- num = min(num, ARRAY_SIZE(vb->pfns));
+ /* Traditionally, we can only do one array worth at a time. */
+ if (!chunking)
+ num = min(num, ARRAY_SIZE(vb->pfns));
mutex_lock(&vb->balloon_lock);
/* We can't release more pages than taken */
@@ -210,7 +479,10 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
page = balloon_page_dequeue(vb_dev_info);
if (!page)
break;
- set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+ if (chunking)
+ update_pfn_range(vb, page, &pfn_max, &pfn_min);
+ else
+ set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
list_add(&page->lru, &pages);
vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
}
@@ -221,8 +493,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
* virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
* is true, we *have* to do it in this order
*/
- if (vb->num_pfns != 0)
- tell_host(vb, vb->deflate_vq);
+ if (vb->num_pfns != 0) {
+ if (chunking)
+ tell_host_page_chunks(vb, &pages, vb->deflate_vq,
+ pfn_max, pfn_min);
+ else
+ tell_host(vb, vb->deflate_vq);
+ }
release_pages_balloon(vb, &pages);
mutex_unlock(&vb->balloon_lock);
return num_freed_pages;
@@ -442,6 +719,14 @@ static int init_vqs(struct virtio_balloon *vb)
}
#ifdef CONFIG_BALLOON_COMPACTION
+
+static void tell_host_one_page(struct virtio_balloon *vb,
+ struct virtqueue *vq, struct page *page)
+{
+ add_one_chunk(vb, vq, page_to_pfn(page),
+ VIRTIO_BALLOON_PAGES_PER_PAGE);
+}
+
/*
* virtballoon_migratepage - perform the balloon page migration on behalf of
* a compation thread. (called under page lock)
@@ -465,6 +750,8 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
{
struct virtio_balloon *vb = container_of(vb_dev_info,
struct virtio_balloon, vb_dev_info);
+ bool chunking = virtio_has_feature(vb->vdev,
+ VIRTIO_BALLOON_F_PAGE_CHUNKS);
unsigned long flags;
/*
@@ -486,16 +773,22 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
vb_dev_info->isolated_pages--;
__count_vm_event(BALLOON_MIGRATE);
spin_unlock_irqrestore(&vb_dev_info->pages_lock, flags);
- vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
- set_page_pfns(vb, vb->pfns, newpage);
- tell_host(vb, vb->inflate_vq);
-
+ if (chunking) {
+ tell_host_one_page(vb, vb->inflate_vq, newpage);
+ } else {
+ vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
+ set_page_pfns(vb, vb->pfns, newpage);
+ tell_host(vb, vb->inflate_vq);
+ }
/* balloon's page migration 2nd step -- deflate "page" */
balloon_page_delete(page);
- vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
- set_page_pfns(vb, vb->pfns, page);
- tell_host(vb, vb->deflate_vq);
-
+ if (chunking) {
+ tell_host_one_page(vb, vb->deflate_vq, page);
+ } else {
+ vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
+ set_page_pfns(vb, vb->pfns, page);
+ tell_host(vb, vb->deflate_vq);
+ }
mutex_unlock(&vb->balloon_lock);
put_page(page); /* balloon reference */
@@ -522,9 +815,75 @@ static struct file_system_type balloon_fs = {
#endif /* CONFIG_BALLOON_COMPACTION */
+static void free_page_bmap(struct virtio_balloon *vb)
+{
+ int i;
+
+ for (i = 0; i < VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i++) {
+ kfree(vb->page_bmap[i]);
+ vb->page_bmap[i] = NULL;
+ }
+}
+
+static int balloon_page_chunk_init(struct virtio_balloon *vb)
+{
+ int i;
+
+ vb->balloon_page_chunk = kmalloc(sizeof(__le64) +
+ sizeof(struct virtio_balloon_page_chunk_entry) *
+ VIRTIO_BALLOON_MAX_PAGE_CHUNKS, GFP_KERNEL);
+ if (!vb->balloon_page_chunk)
+ goto err_page_chunk;
+
+ /*
+ * The default number of page_bmaps are allocated. More may be
+ * allocated on demand.
+ */
+ for (i = 0; i < VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i++) {
+ vb->page_bmap[i] = kmalloc(VIRTIO_BALLOON_PAGE_BMAP_SIZE,
+ GFP_KERNEL);
+ if (!vb->page_bmap[i])
+ goto err_page_bmap;
+ }
+
+ return 0;
+err_page_bmap:
+ free_page_bmap(vb);
+ kfree(vb->balloon_page_chunk);
+ vb->balloon_page_chunk = NULL;
+err_page_chunk:
+ __virtio_clear_bit(vb->vdev, VIRTIO_BALLOON_F_PAGE_CHUNKS);
+ dev_warn(&vb->vdev->dev, "%s: failed\n", __func__);
+ return -ENOMEM;
+}
+
+static int virtballoon_validate(struct virtio_device *vdev)
+{
+ struct virtio_balloon *vb = NULL;
+ int err;
+
+ vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
+ if (!vb) {
+ err = -ENOMEM;
+ goto err_vb;
+ }
+
+ if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_CHUNKS)) {
+ err = balloon_page_chunk_init(vb);
+ if (err < 0)
+ goto err_page_chunk;
+ }
+
+ return 0;
+err_page_chunk:
+ kfree(vb);
+err_vb:
+ return err;
+}
+
static int virtballoon_probe(struct virtio_device *vdev)
{
- struct virtio_balloon *vb;
+ struct virtio_balloon *vb = vdev->priv;
int err;
if (!vdev->config->get) {
@@ -533,17 +892,12 @@ static int virtballoon_probe(struct virtio_device *vdev)
return -EINVAL;
}
- vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
- if (!vb) {
- err = -ENOMEM;
- goto out;
- }
-
INIT_WORK(&vb->update_balloon_stats_work, update_balloon_stats_func);
INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
spin_lock_init(&vb->stop_update_lock);
vb->stop_update = false;
vb->num_pages = 0;
+
mutex_init(&vb->balloon_lock);
init_waitqueue_head(&vb->acked);
vb->vdev = vdev;
@@ -590,7 +944,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
vdev->config->del_vqs(vdev);
out_free_vb:
kfree(vb);
-out:
return err;
}
@@ -620,6 +973,8 @@ static void virtballoon_remove(struct virtio_device *vdev)
cancel_work_sync(&vb->update_balloon_stats_work);
remove_common(vb);
+ free_page_bmap(vb);
+ kfree(vb->balloon_page_chunk);
#ifdef CONFIG_BALLOON_COMPACTION
if (vb->vb_dev_info.inode)
iput(vb->vb_dev_info.inode);
@@ -664,6 +1019,7 @@ static unsigned int features[] = {
VIRTIO_BALLOON_F_MUST_TELL_HOST,
VIRTIO_BALLOON_F_STATS_VQ,
VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
+ VIRTIO_BALLOON_F_PAGE_CHUNKS,
};
static struct virtio_driver virtio_balloon_driver = {
@@ -674,6 +1030,7 @@ static struct virtio_driver virtio_balloon_driver = {
.id_table = id_table,
.probe = virtballoon_probe,
.remove = virtballoon_remove,
+ .validate = virtballoon_validate,
.config_changed = virtballoon_changed,
#ifdef CONFIG_PM_SLEEP
.freeze = virtballoon_freeze,
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 343d7dd..d532ed16 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -34,6 +34,7 @@
#define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
#define VIRTIO_BALLOON_F_STATS_VQ 1 /* Memory Stats virtqueue */
#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
+#define VIRTIO_BALLOON_F_PAGE_CHUNKS 3 /* Inflate/Deflate pages in chunks */
/* Size of a PFN in the balloon interface. */
#define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -82,4 +83,17 @@ struct virtio_balloon_stat {
__virtio64 val;
} __attribute__((packed));
+#define VIRTIO_BALLOON_CHUNK_BASE_SHIFT 12
+#define VIRTIO_BALLOON_CHUNK_SIZE_SHIFT 12
+struct virtio_balloon_page_chunk_entry {
+ __le64 base;
+ __le64 size;
+};
+
+struct virtio_balloon_page_chunk {
+ /* Number of chunks in the payload */
+ __le64 chunk_num;
+ struct virtio_balloon_page_chunk_entry entry[];
+};
+
#endif /* _LINUX_VIRTIO_BALLOON_H */
--
2.7.4
^ permalink raw reply related
* [PATCH v10 2/6] virtio-balloon: coding format cleanup
From: Wei Wang @ 2017-05-04 8:50 UTC (permalink / raw)
To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1493887815-6070-1-git-send-email-wei.w.wang@intel.com>
Clean up the comment format.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
drivers/virtio/virtio_balloon.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 4a9f307..ecb64e9 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -132,8 +132,10 @@ static void set_page_pfns(struct virtio_balloon *vb,
{
unsigned int i;
- /* Set balloon pfns pointing at this page.
- * Note that the first pfn points at start of the page. */
+ /*
+ * Set balloon pfns pointing at this page.
+ * Note that the first pfn points at start of the page.
+ */
for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
pfns[i] = cpu_to_virtio32(vb->vdev,
page_to_balloon_pfn(page) + i);
--
2.7.4
^ permalink raw reply related
* [PATCH v10 1/6] virtio-balloon: deflate via a page list
From: Wei Wang @ 2017-05-04 8:50 UTC (permalink / raw)
To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1493887815-6070-1-git-send-email-wei.w.wang@intel.com>
From: Liang Li <liang.z.li@intel.com>
This patch saves the deflated pages to a list, instead of the PFN array.
Accordingly, the balloon_pfn_to_page() function is removed.
Signed-off-by: Liang Li <liang.z.li@intel.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
drivers/virtio/virtio_balloon.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 34adf9b..4a9f307 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -104,12 +104,6 @@ static u32 page_to_balloon_pfn(struct page *page)
return pfn * VIRTIO_BALLOON_PAGES_PER_PAGE;
}
-static struct page *balloon_pfn_to_page(u32 pfn)
-{
- BUG_ON(pfn % VIRTIO_BALLOON_PAGES_PER_PAGE);
- return pfn_to_page(pfn / VIRTIO_BALLOON_PAGES_PER_PAGE);
-}
-
static void balloon_ack(struct virtqueue *vq)
{
struct virtio_balloon *vb = vq->vdev->priv;
@@ -182,18 +176,16 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
return num_allocated_pages;
}
-static void release_pages_balloon(struct virtio_balloon *vb)
+static void release_pages_balloon(struct virtio_balloon *vb,
+ struct list_head *pages)
{
- unsigned int i;
- struct page *page;
+ struct page *page, *next;
- /* Find pfns pointing at start of each page, get pages and free them. */
- for (i = 0; i < vb->num_pfns; i += VIRTIO_BALLOON_PAGES_PER_PAGE) {
- page = balloon_pfn_to_page(virtio32_to_cpu(vb->vdev,
- vb->pfns[i]));
+ list_for_each_entry_safe(page, next, pages, lru) {
if (!virtio_has_feature(vb->vdev,
VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
adjust_managed_page_count(page, 1);
+ list_del(&page->lru);
put_page(page); /* balloon reference */
}
}
@@ -203,6 +195,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
unsigned num_freed_pages;
struct page *page;
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
+ LIST_HEAD(pages);
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
@@ -216,6 +209,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
if (!page)
break;
set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+ list_add(&page->lru, &pages);
vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
}
@@ -227,7 +221,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
*/
if (vb->num_pfns != 0)
tell_host(vb, vb->deflate_vq);
- release_pages_balloon(vb);
+ release_pages_balloon(vb, &pages);
mutex_unlock(&vb->balloon_lock);
return num_freed_pages;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v10 0/6] Extend virtio-balloon for fast (de)inflating & fast live migration
From: Wei Wang @ 2017-05-04 8:50 UTC (permalink / raw)
To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
This patch series implements the follow two things:
1) Optimization of balloon page transfer: instead of transferring balloon pages
to host one by one, the new mechanism transfers them in chunks.
2) A mechanism to report info of guest unused pages: the pages have been unused
at some time between when host sent command and when guest reported them. Host
uses that by tracking memory changes and then discarding changes made to the
pages that it gets from guest before it sent the command.
Changes:
v9->v10:
1) mm: put report_unused_page_block() under CONFIG_VIRTIO_BALLOON;
2) virtio-balloon: add virtballoon_validate();
3) virtio-balloon: msg format change;
4) virtio-balloon: move miscq handling to a task on system_freezable_wq;
5) virtio-balloon: code cleanup.
v8->v9:
1) Split the two new features, VIRTIO_BALLOON_F_BALLOON_CHUNKS and
VIRTIO_BALLOON_F_MISC_VQ, which were mixed together in the previous
implementation;
2) Simpler function to get the free page block.
v7->v8:
1) Use only one chunk format, instead of two.
2) re-write the virtio-balloon implementation patch.
3) commit changes
4) patch re-org
Liang Li (1):
virtio-balloon: deflate via a page list
Wei Wang (5):
virtio-balloon: coding format cleanup
virtio-balloon: VIRTIO_BALLOON_F_PAGE_CHUNKS
mm: function to offer a page block on the free list
mm: export symbol of next_zone and first_online_pgdat
virtio-balloon: VIRTIO_BALLOON_F_MISC_VQ
drivers/virtio/virtio_balloon.c | 696 +++++++++++++++++++++++++++++++++---
include/linux/mm.h | 5 +
include/uapi/linux/virtio_balloon.h | 26 ++
mm/mmzone.c | 2 +
mm/page_alloc.c | 91 +++++
5 files changed, 761 insertions(+), 59 deletions(-)
--
2.7.4
^ permalink raw reply
* CFP: KVM Forum 2017
From: Paolo Bonzini @ 2017-05-02 10:41 UTC (permalink / raw)
To: KVM list, linux-kernel@vger.kernel.org, Linux Virtualization
================================================================
KVM Forum 2017: Call For Participation
October 25-27, 2017 - Hilton Prague - Prague, Czech Republic
(All submissions must be received before midnight June 15, 2017)
=================================================================
KVM Forum is an annual event that presents a rare opportunity
for developers and users to meet, discuss the state of Linux
virtualization technology, and plan for the challenges ahead.
We invite you to lead part of the discussion by submitting a speaking
proposal for KVM Forum 2017.
At this highly technical conference, developers driving innovation
in the KVM virtualization stack (Linux, KVM, QEMU, libvirt) can
meet users who depend on KVM as part of their offerings, or to
power their data centers and clouds.
KVM Forum will include sessions on the state of the KVM
virtualization stack, planning for the future, and many
opportunities for attendees to collaborate. As we celebrate ten years
of KVM development in the Linux kernel, KVM continues to be a
critical part of the FOSS cloud infrastructure.
This year, KVM Forum is joining Open Source Summit in Prague,
Czech Republic. Selected talks from KVM Forum will be presented on
Wednesday October 25 to the full audience of the Open Source Summit.
Also, attendees of KVM Forum will have access to all of the talks from
Open Source Summit on Wednesday.
http://events.linuxfoundation.org/cfp
Suggested topics:
* Scaling, latency optimizations, performance tuning, real-time guests
* Hardening and security
* New features
* Testing
KVM and the Linux kernel:
* Nested virtualization
* Resource management (CPU, I/O, memory) and scheduling
* VFIO: IOMMU, SR-IOV, virtual GPU, etc.
* Networking: Open vSwitch, XDP, etc.
* virtio and vhost
* Architecture ports and new processor features
QEMU:
* Management interfaces: QOM and QMP
* New devices, new boards, new architectures
* Graphics, desktop virtualization and virtual GPU
* New storage features
* High availability, live migration and fault tolerance
* Emulation and TCG
* Firmware: ACPI, UEFI, coreboot, U-Boot, etc.
Management and infrastructure
* Managing KVM: Libvirt, OpenStack, oVirt, etc.
* Storage: Ceph, Gluster, SPDK, etc.r
* Network Function Virtualization: DPDK, OPNFV, OVN, etc.
* Provisioning
===============
SUBMITTING YOUR PROPOSAL
===============
Abstracts due: June 15, 2017
Please submit a short abstract (~150 words) describing your presentation
proposal. Slots vary in length up to 45 minutes. Also include the proposal
type -- one of:
- technical talk
- end-user talk
Submit your proposal here:
http://events.linuxfoundation.org/cfp
Please only use the categories "presentation" and "panel discussion"
You will receive a notification whether or not your presentation proposal
was accepted by August 10, 2017.
Speakers will receive a complimentary pass for the event. In the instance
that case your submission has multiple presenters, only the primary speaker for a
proposal will receive a complimentary event pass. For panel discussions, all
panelists will receive a complimentary event pass.
TECHNICAL TALKS
A good technical talk should not just report on what has happened over
the last year; it should present a concrete problem and how it impacts
the user and/or developer community. Whenever applicable, focus on
work that needs to be done, difficulties that haven't yet been solved,
and on decisions that other developers should be aware of. Summarizing
recent developments is okay but it should not be more than a small
portion of the overall talk.
END-USER TALKS
One of the big challenges as developers is to know what, where and how
people actually use our software. We will reserve a few slots for end
users talking about their deployment challenges and achievements.
If you are using KVM in production you are encouraged submit a speaking
proposal. Simply mark it as an end-user talk. As an end user, this is a
unique opportunity to get your input to developers.
HANDS-ON / BOF SESSIONS
We will reserve some time for people to get together and discuss
strategic decisions as well as other topics that are best solved within
smaller groups.
These sessions will be announced during the event. If you are interested
in organizing such a session, please add it to the list at
http://www.linux-kvm.org/page/KVM_Forum_2017_BOF
Let people you think who might be interested know about your BOF, and encourage
them to add their names to the wiki page as well. Please try to
add your ideas to the list before KVM Forum starts.
PANEL DISCUSSIONS
If you are proposing a panel discussion, please make sure that you list
all of your potential panelists in your the abstract. We will request full
biographies if a panel is accepted.
===============
HOTEL / TRAVEL
===============
This year's event will take place at the Hilton Prague.
For information on discounted room rates for conference attendees
and on other hotels close to the conference, please visit
http://events.linuxfoundation.org/events/kvm-forum/attend/hotel-travel.
===============
IMPORTANT DATES
===============
Submission deadline: June 15, 2017
Notification: August 10, 2017
Schedule announced: August 17, 2017
Event dates: October 25-27, 2017
Thank you for your interest in KVM. We're looking forward to your
submissions and seeing you at the KVM Forum 2017 in October!
-your KVM Forum 2017 Program Committee
Please contact us with any questions or comments at
kvm-forum-2017-pc@redhat.com
^ permalink raw reply
* Re: [PATCH] tools/virtio: fix spelling mistake: "wakeus" -> "wakeups"
From: Jason Wang @ 2017-05-02 6:01 UTC (permalink / raw)
To: Colin King, Michael S . Tsirkin, virtualization; +Cc: linux-kernel
In-Reply-To: <20170429221421.12238-1-colin.king@canonical.com>
On 2017年04月30日 06:14, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> trivial fix to spelling mistake in an error message.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> tools/virtio/virtio_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> index 76d6f583c249..0fecaec90d0d 100644
> --- a/tools/virtio/virtio_test.c
> +++ b/tools/virtio/virtio_test.c
> @@ -202,7 +202,7 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
> test = 0;
> r = ioctl(dev->control, VHOST_TEST_RUN, &test);
> assert(r >= 0);
> - fprintf(stderr, "spurious wakeus: 0x%llx\n", spurious);
> + fprintf(stderr, "spurious wakeups: 0x%llx\n", spurious);
> }
>
> const char optstring[] = "h";
Acked-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH] tools/virtio: fix spelling mistake: "wakeus" -> "wakeups"
From: Colin King @ 2017-04-29 22:14 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, virtualization; +Cc: linux-kernel
From: Colin Ian King <colin.king@canonical.com>
trivial fix to spelling mistake in an error message.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
tools/virtio/virtio_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 76d6f583c249..0fecaec90d0d 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -202,7 +202,7 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
test = 0;
r = ioctl(dev->control, VHOST_TEST_RUN, &test);
assert(r >= 0);
- fprintf(stderr, "spurious wakeus: 0x%llx\n", spurious);
+ fprintf(stderr, "spurious wakeups: 0x%llx\n", spurious);
}
const char optstring[] = "h";
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v9 5/5] virtio-balloon: VIRTIO_BALLOON_F_MISC_VQ
From: Wei Wang @ 2017-04-27 6:33 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, dave.hansen, linux-kernel, virtualization,
linux-mm, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <20170413194732-mutt-send-email-mst@kernel.org>
On 04/14/2017 01:08 AM, Michael S. Tsirkin wrote:
> On Thu, Apr 13, 2017 at 05:35:08PM +0800, Wei Wang wrote:
>> Add a new vq, miscq, to handle miscellaneous requests between the device
>> and the driver.
>>
>> This patch implemnts the VIRTIO_BALLOON_MISCQ_INQUIRE_UNUSED_PAGES
> implements
>
>> request sent from the device.
> Commands are sent from host and handled on guest.
> In fact how is this so different from stats?
> How about reusing the stats vq then? You can use one buffer
> for stats and one buffer for commands.
>
The meaning of the two vqs is a little different. statq is used for
reporting statistics, while miscq is intended to be used to handle
miscellaneous requests from the guest or host (I think it can
also be used the other way around in the future when other
new features are added which need the guest to send requests
and the host to provide responses).
I would prefer to have them separate, because:
If we plan to combine them, we need to put the previous statq
related implementation under miscq with a new command (I think
we can't combine them without using commands to distinguish
the two features).
In this way, an old driver won't work with a new QEMU or a new
driver won't work with an old QEMU. Would this be considered
as an issue here?
>
>> + miscq_out_hdr->flags = 0;
>> +
>> + for_each_populated_zone(zone) {
>> + for (order = MAX_ORDER - 1; order > 0; order--) {
>> + for (migratetype = 0; migratetype < MIGRATE_TYPES;
>> + migratetype++) {
>> + do {
>> + ret = inquire_unused_page_block(zone,
>> + order, migratetype, &page);
>> + if (!ret) {
>> + pfn = (u64)page_to_pfn(page);
>> + add_one_chunk(vb, vq,
>> + PAGE_CHUNK_TYPE_UNUSED,
>> + pfn,
>> + (u64)(1 << order));
>> + }
>> + } while (!ret);
>> + }
>> + }
>> + }
>> + miscq_out_hdr->flags |= VIRTIO_BALLOON_MISCQ_F_COMPLETE;
> And where is miscq_out_hdr used? I see no add_outbuf anywhere.
>
> Things like this should be passed through function parameters
> and not stuffed into device structure, fields should be
> initialized before use and not where we happen to
> have the data handy.
>
miscq_out_hdr is linear with the payload (i.e. kmalloc(hdr+payload) ).
It is the same as the use of statq - one request in-flight each time.
>
> Also, _F_ is normally a bit number, you use it as a value here.
>
It intends to be a bit number. Bit 0 of flags to indicate the completion
of handling the request.
^ permalink raw reply
* Re: [virtio-dev] Re: [PATCH v9 2/5] virtio-balloon: VIRTIO_BALLOON_F_BALLOON_CHUNKS
From: Wei Wang @ 2017-04-27 6:31 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, Hansen, Dave,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <20170426192753-mutt-send-email-mst@kernel.org>
On 04/27/2017 07:20 AM, Michael S. Tsirkin wrote:
> On Wed, Apr 26, 2017 at 11:03:34AM +0000, Wang, Wei W wrote:
>> Hi Michael, could you please give some feedback?
> I'm sorry, I'm not sure feedback on what you are requesting.
Oh, just some trivial things (e.g. use a field in the
header, hdr->chunks to indicate the number of chunks
in the payload) that wasn't confirmed.
I will prepare the new version with fixing the agreed issues,
and we can continue to discuss those parts if you still find
them improper.
>
> The interface looks reasonable now, even though there's
> a way to make it even simpler if we can limit chunk size
> to 2G (in fact 4G - 1). Do you think we can live with this
> limitation?
Yes, I think we can. So, is it good to change to use the
previous 64-bit chunk format (52-bit base + 12-bit size)?
>
> But the code still needs some cleanup.
>
OK. We'll also still to discuss your comments in the patch 05.
Best,
Wei
^ permalink raw reply
* Re: [virtio-dev] Re: [PATCH v9 2/5] virtio-balloon: VIRTIO_BALLOON_F_BALLOON_CHUNKS
From: Michael S. Tsirkin @ 2017-04-26 23:20 UTC (permalink / raw)
To: Wang, Wei W
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, Hansen, Dave,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <286AC319A985734F985F78AFA26841F7391F6DCD@shsmsx102.ccr.corp.intel.com>
On Wed, Apr 26, 2017 at 11:03:34AM +0000, Wang, Wei W wrote:
> Hi Michael, could you please give some feedback?
I'm sorry, I'm not sure feedback on what you are requesting.
The interface looks reasonable now, even though there's
a way to make it even simpler if we can limit chunk size
to 2G (in fact 4G - 1). Do you think we can live with this
limitation?
But the code still needs some cleanup.
--
MST
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: on tx, only call napi_disable if tx napi is on
From: David Miller @ 2017-04-26 18:50 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, virtualization, willemb, mst
In-Reply-To: <20170425195917.54209-1-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Tue, 25 Apr 2017 15:59:17 -0400
> From: Willem de Bruijn <willemb@google.com>
>
> As of tx napi, device down (`ip link set dev $dev down`) hangs unless
> tx napi is enabled. Else napi_enable is not called, so napi_disable
> will spin on test_and_set_bit NAPI_STATE_SCHED.
>
> Only call napi_disable if tx napi is enabled.
>
> Fixes: 5a719c2552ca ("virtio-net: transmit napi")
> Reported-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [RFC 3/3] virtio-iommu: future work
From: Michael S. Tsirkin @ 2017-04-26 16:24 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: virtio-dev, lorenzo.pieralisi, kvm, cdall, marc.zyngier, joro,
will.deacon, virtualization, iommu, robin.murphy
In-Reply-To: <20170407191747.26618-4-jean-philippe.brucker@arm.com>
On Fri, Apr 07, 2017 at 08:17:47PM +0100, Jean-Philippe Brucker wrote:
> Here I propose a few ideas for extensions and optimizations. This is all
> very exploratory, feel free to correct mistakes and suggest more things.
>
> I. Linux host
> 1. vhost-iommu
A qemu based implementation would be a first step.
Would allow validating the claim that it's much
simpler to support than e.g. VTD.
> 2. VFIO nested translation
> II. Page table sharing
> 1. Sharing IOMMU page tables
> 2. Sharing MMU page tables (SVM)
> 3. Fault reporting
> 4. Host implementation with VFIO
> III. Relaxed operations
> IV. Misc
>
>
> I. Linux host
> =============
>
> 1. vhost-iommu
> --------------
>
> An advantage of virtualizing an IOMMU using virtio is that it allows to
> hoist a lot of the emulation code into the kernel using vhost, and avoid
> returning to userspace for each request. The mainline kernel already
> implements vhost-net, vhost-scsi and vhost-vsock, and a lot of core code
> could be reused.
>
> Introducing vhost in a simplified scenario 1 (removed guest userspace
> pass-through, irrelevant to this example) gives us the following:
>
> MEM____pIOMMU________PCI device____________ HARDWARE
> | \
> ----------|-------------+-------------+-----\--------------------------
> | : KVM : \
> pIOMMU drv : : \ KERNEL
> | : : net drv
> VFIO : : /
> | : : /
> vhost-iommu_________________________virtio-iommu-drv
> : :
> --------------------------------------+-------------------------------
> HOST : GUEST
>
>
> Introducing vhost in scenario 2, userspace now only handles the device
> initialisation part, and most runtime communication is handled in kernel:
>
> MEM__pIOMMU___PCI device HARDWARE
> | |
> -------|---------|------+-------------+-------------------------------
> | | : KVM :
> pIOMMU drv | : : KERNEL
> \__net drv : :
> | : :
> tap : :
> | : :
> _vhost-net________________________virtio-net drv
> (2) / : : / (1a)
> / : : /
> vhost-iommu________________________________virtio-iommu drv
> : : (1b)
> ------------------------+-------------+-------------------------------
> HOST : GUEST
>
> (1) a. Guest virtio driver maps ring and buffers
> b. Map requests are relayed to the host the same way.
> (2) To access any guest memory, vhost-net must query the IOMMU. We can
> reuse the existing TLB protocol for this. TLB commands are written to
> and read from the vhost-net fd.
>
> As defined in Linux/include/uapi/linux/vhost.h, the vhost msg structure
> has everything needed for map/unmap operations:
>
> struct vhost_iotlb_msg {
> __u64 iova;
> __u64 size;
> __u64 uaddr;
> __u8 perm; /* R/W */
> __u8 type;
> #define VHOST_IOTLB_MISS
> #define VHOST_IOTLB_UPDATE /* MAP */
> #define VHOST_IOTLB_INVALIDATE /* UNMAP */
> #define VHOST_IOTLB_ACCESS_FAIL
> };
>
> struct vhost_msg {
> int type;
> union {
> struct vhost_iotlb_msg iotlb;
> __u8 padding[64];
> };
> };
>
> The vhost-iommu device associates a virtual device ID to a TLB fd. We
> should be able to use the same commands for [vhost-net <-> virtio-iommu]
> and [virtio-net <-> vhost-iommu] communication. A virtio-net device
> would open a socketpair and hand one side to vhost-iommu.
>
> If vhost_msg is ever used for another purpose than TLB, we'll have some
> trouble, as there will be multiple clients that want to read/write the
> vhost fd. A multicast transport method will be needed. Until then, this
> can work.
>
> Details of operations would be:
>
> (1) Userspace sets up vhost-iommu as with other vhost devices, by using
> standard vhost ioctls. Userspace starts by describing the system topology
> via ioctl:
>
> ioctl(iommu_fd, VHOST_IOMMU_ADD_DEVICE, struct
> vhost_iommu_add_device)
>
> #define VHOST_IOMMU_DEVICE_TYPE_VFIO
> #define VHOST_IOMMU_DEVICE_TYPE_TLB
>
> struct vhost_iommu_add_device {
> __u8 type;
> __u32 devid;
> union {
> struct vhost_iommu_device_vfio {
> int vfio_group_fd;
> };
> struct vhost_iommu_device_tlb {
> int fd;
> };
> };
> };
>
> (2) VIRTIO_IOMMU_T_ATTACH(address space, devid)
>
> vhost-iommu creates an address space if necessary, finds the device along
> with the relevant operations. If type is VFIO, operations are done on a
> container, otherwise they are done on single devices.
>
> (3) VIRTIO_IOMMU_T_MAP(address space, virt, phys, size, flags)
>
> Turn phys into an hva using the vhost mem table.
>
> - If type is TLB, either preload with VHOST_IOTLB_UPDATE or store the
> mapping locally and wait for the TLB to ask for it with a
> VHOST_IOTLB_MISS.
> - If type is VFIO, turn it into a VFIO_IOMMU_MAP_DMA (might need to
> introduce a shortcut in the external user API of VFIO).
>
> (4) VIRTIO_IOMMU_T_UNMAP(address space, virt, phys, size, flags)
>
> - If type is TLB, send a VHOST_IOTLB_INVALIDATE.
> - If type is VFIO, turn it into VFIO_IOMMU_UNMAP_DMA.
>
> (5) VIRTIO_IOMMU_T_DETACH(address space, devid)
>
> Undo whatever was done in (2).
>
>
> 2. VFIO nested translation
> --------------------------
>
> For my current kvmtool implementation, I am putting each VFIO group in a
> different container during initialization. We cannot detach a group from a
> container at runtime without first resetting all devices in that group. So
> the best way to provide dynamic address spaces right now is one container
> per group. The drawback is that we need to maintain multiple sets of page
> tables even if the guest wants to put all devices in the same address
> space. Another disadvantage is when implementing bypass mode, we need to
> map the whole address space at the beginning, then unmap everything on
> attach. Adding nested support would be a nice way to provide dynamic
> address spaces while keeping groups tied to a container at all times.
>
> A physical IOMMU may offer nested translation. In this case, address
> spaces are managed by two page directories instead of one. A guest-
> virtual address is translated into a guest-physical one using what we'll
> call here "stage-1" (s1) page tables, and the guest-physical address is
> translated into a host-physical one using "stage-2" (s2) page tables.
>
> s1 s2
> GVA --> GPA --> HPA
>
> There isn't a lot of support in Linux for nesting IOMMU page directories
> at the moment (though SVM support is coming, see II). VFIO does have a
> "nesting" IOMMU type, which doesn't mean much at the moment. The ARM SMMU
> code uses this to decide whether to manage the container with s2 page
> tables instead of s1, but even then we still only have a single stage and
> it is assumed that IOVA=GPA.
>
> Another model that would help with dynamically changing address spaces is
> nesting VFIO containers:
>
> Parent <---------- map/unmap
> container
> / | \
> / group \
> Child Child <--- map/unmap
> container container
> | | |
> group group group
>
> At the beginning all groups are attached to the parent container, and
> there is no child container. Doing map/unmap on the parent container maps
> stage-2 page tables (map GPA -> HVA and pin the page -> HPA). User should
> be able to choose whether they want all devices attached to this container
> to be able to access GPAs (bypass mode, as it currently is) or simply
> block all DMA (in which case there is no need to pin pages here).
>
> At some point the guest wants to create an address space and attaches
> children to it. Using an ioctl (to be defined), we can derive a child
> container from the parent container, and move groups from parent to child.
>
> This returns a child fd. When the guest maps something in this new address
> space, we can do a map ioctl on the child container, which maps stage-1
> page tables (map GVA -> GPA).
>
> A page table walk may access multiple levels of tables (pgd, p4d, pud,
> pmd, pt). With nested translation, each access to a table during the
> stage-1 walk requires a stage-2 walk. This makes a full translation costly
> so it is preferable to use a single stage of translation when possible.
> Folding two stages into one is simple with a single container, as shown in
> the kvmtool example. The host keeps track of GPA->HVA mappings, so it can
> fold the full GVA->HVA mapping before sending the VFIO request. With
> nested containers however, the IOMMU driver would have to do the folding
> work itself. Keeping a copy of stage-2 mapping created on the parent
> container, it would fold them into the actual stage-2 page tables when
> receiving a map request on the child container (note that software folding
> is not possible when stage-1 pgd is managed by the guest, as described in
> next section).
>
> I don't know if nested VFIO containers are a desirable feature at all. I
> find the concept cute on paper, and it would make it easier for userspace
> to juggle with address spaces, but it might require some invasive changes
> in VFIO, and people have been able to use the current API for IOMMU
> virtualization so far.
>
>
> II. Page table sharing
> ======================
>
> 1. Sharing IOMMU page tables
> ----------------------------
>
> VIRTIO_IOMMU_F_PT_SHARING
>
> This is independent of the nested mode described in I.2, but relies on a
> similar feature in the physical IOMMU: having two stages of page tables,
> one for the host and one for the guest.
>
> When this is supported, the guest can manage its own s1 page directory, to
> avoid sending MAP/UNMAP requests. Feature VIRTIO_IOMMU_F_PT_SHARING allows
> a driver to give a page directory pointer (pgd) to the host and send
> invalidations when removing or changing a mapping. In this mode, three
> requests are used: probe, attach and invalidate. An address space cannot
> be using the MAP/UNMAP interface and PT_SHARING at the same time.
>
> Device and driver first need to negotiate which page table format they
> will be using. This depends on the physical IOMMU, so the request contains
> a negotiation part to probe the device capabilities.
>
> (1) Driver attaches devices to address spaces as usual, but a flag
> VIRTIO_IOMMU_ATTACH_F_PRIVATE (working title) tells the device not to
> create page tables for use with the MAP/UNMAP API. The driver intends
> to manage the address space itself.
>
> (2) Driver sends a PROBE_TABLE request. It sets len > 0 with the size of
> pg_format array.
>
> VIRTIO_IOMMU_T_PROBE_TABLE
>
> struct virtio_iommu_req_probe_table {
> le32 address_space;
> le32 flags;
> le32 len;
>
> le32 nr_contexts;
> struct {
> le32 model;
> u8 format[64];
> } pg_format[len];
> };
>
> Introducing a probe request is more flexible than advertising those
> features in virtio config, because capabilities are dynamic, and depend on
> which devices are attached to an address space. Within a single address
> space, devices may support different numbers of contexts (PASIDs), and
> some may not support recoverable faults.
>
> (3) Device responds success with all page table formats implemented by the
> physical IOMMU in pg_format. 'model' 0 is invalid, so driver can
> initialize the array to 0 and deduce from there which entries have
> been filled by the device.
>
> Using a probe method seems preferable over trying to attach every possible
> format until one sticks. For instance, with an ARM guest running on an x86
> host, PROBE_TABLE would return the Intel IOMMU page table format, and the
> guest could use that page table code to handle its mappings, hidden behind
> the IOMMU API. This requires that the page-table code is reasonably
> abstracted from the architecture, as is done with drivers/iommu/io-pgtable
> (an x86 guest could use any format implement by io-pgtable for example.)
>
> (4) If the driver is able to use this format, it sends the ATTACH_TABLE
> request.
>
> VIRTIO_IOMMU_T_ATTACH_TABLE
>
> struct virtio_iommu_req_attach_table {
> le32 address_space;
> le32 flags;
> le64 table;
>
> le32 nr_contexts;
> /* Page-table format description */
>
> le32 model;
> u8 config[64]
> };
>
>
> 'table' is a pointer to the page directory. 'nr_contexts' isn't used
> here.
>
> For both ATTACH and PROBE, 'flags' are the following (and will be
> explained later):
>
> VIRTIO_IOMMU_ATTACH_TABLE_F_INDIRECT (1 << 0)
> VIRTIO_IOMMU_ATTACH_TABLE_F_NATIVE (1 << 1)
> VIRTIO_IOMMU_ATTACH_TABLE_F_FAULT (1 << 2)
>
> Now 'model' is a bit tricky. We need to specify all possible page table
> formats and their parameters. I'm not well-versed in x86, s390 or other
> IOMMUs, so I'll just focus on the ARM world for this example. We basically
> have two page table models, with a multitude of configuration bits:
>
> * ARM LPAE
> * ARM short descriptor
>
> We could define a high-level identifier per page-table model, such as:
>
> #define PG_TABLE_ARM 0x1
> #define PG_TABLE_X86 0x2
> ...
>
> And each model would define its own structure. On ARM 'format' could be a
> simple u32 defining a variant, LPAE 32/64 or short descriptor. It could
> also contain additional capabilities. Then depending on the variant,
> 'config' would be:
>
> struct pg_config_v7s {
> le32 tcr;
> le32 prrr;
> le32 nmrr;
> le32 asid;
> };
>
> struct pg_config_lpae {
> le64 tcr;
> le64 mair;
> le32 asid;
>
> /* And maybe TTB1? */
> };
>
> struct pg_config_arm {
> le32 variant;
> union ...;
> };
>
> I am really uneasy with describing all those nasty architectural details
> in the virtio-iommu specification. We certainly won't start describing the
> content bit-by-bit of tcr or mair here, but just declaring these fields
> might be sufficient.
>
> (5) Once the table is attached, the driver can simply write the page
> tables and expect the physical IOMMU to observe the mappings without
> any additional request. When changing or removing a mapping, however,
> the driver must send an invalidate request.
>
> VIRTIO_IOMMU_T_INVALIDATE
>
> struct virtio_iommu_req_invalidate {
> le32 address_space;
> le32 context;
> le32 flags;
> le64 virt_addr;
> le64 range_size;
>
> u8 opaque[64];
> };
>
> 'flags' may be:
>
> VIRTIO_IOMMU_INVALIDATE_T_VADDR: invalidate a single VA range
> from 'context' (context is 0 when !F_INDIRECT).
>
> And with context tables only (explained below):
>
> VIRTIO_IOMMU_INVALIDATE_T_SINGLE: invalidate all mappings from
> 'context' (context is 0 when !F_INDIRECT). virt_addr and range_size
> are ignored.
>
> VIRTIO_IOMMU_INVALIDATE_T_TABLE: with F_INDIRECT, invalidate entries
> in the table that changed. Device reads the table again, compares it
> to previous values, and invalidate all mappings for contexts that
> changed. context, virt_addr and range_size are ignored.
>
> IOMMUs may offer hints and quirks in their invalidation packets. The
> opaque structure in invalidate would allow to transport those. This
> depends on the page table format and as with architectural page-table
> definitions, I really don't want to have those details in the spec itself.
>
>
> 2. Sharing MMU page tables
> --------------------------
>
> The guest can share process page-tables with the physical IOMMU. To do
> that, it sends PROBE_TABLE with (F_INDIRECT | F_NATIVE | F_FAULT). The
> page table format is implicit, so the pg_format array can be empty (unless
> the guest wants to query some specific property, e.g. number of levels
> supported by the pIOMMU?). If the host answers with success, guest can
> send its MMU page table details with ATTACH_TABLE and (F_NATIVE |
> F_INDIRECT | F_FAULT) flags.
>
> F_FAULT means that the host communicates page requests from device to the
> guest, and the guest can handle them by mapping virtual address in the
> fault to pages. It is only available with VIRTIO_IOMMU_F_FAULT_QUEUE (see
> below.)
>
> F_NATIVE means that the pIOMMU pgtable format is the same as guest MMU
> pgtable format.
>
> F_INDIRECT means that 'table' pointer is a context table, instead of a
> page directory. Each slot in the context table points to a page directory:
>
> 64 2 1 0
> table ----> +---------------------+
> | pgd |0|1|<--- context 0
> | --- |0|0|<--- context 1
> | pgd |0|1|
> | --- |0|0|
> | --- |0|0|
> +---------------------+
> | \___Entry is valid
> |______reserved
>
> Question: do we want per-context page table format, or can it stay global
> for the whole indirect table?
>
> Having a context table allows to provide multiple address spaces for a
> single device. In the simplest form, without F_INDIRECT we have a single
> address space per device, but some devices may implement more, for
> instance devices with the PCI PASID extension.
>
> A slot's position in the context table gives an ID, between 0 and
> nr_contexts. The guest can use this ID to have the device target a
> specific address space with DMA. The mechanism to do that is
> device-specific. For a PCI device, the ID is a PASID, and PCI doesn't
> define a specific way of using them for DMA, it's the device driver's
> concern.
>
>
> 3. Fault reporting
> ------------------
>
> VIRTIO_IOMMU_F_EVENT_QUEUE
>
> With this feature, an event virtqueue (1) is available. For now it will
> only be used for fault handling, but I'm calling it eventq so that other
> asynchronous features can piggy-back on it. Device may report faults and
> page requests by sending buffers via the used ring.
>
> #define VIRTIO_IOMMU_T_FAULT 0x05
>
> struct virtio_iommu_evt_fault {
> struct virtio_iommu_evt_head {
> u8 type;
> u8 reserved[3];
> };
>
> u32 address_space;
> u32 context;
>
> u64 vaddr;
> u32 flags; /* Access details: R/W/X */
>
> /* In the reply: */
> u32 reply; /* Fault handled, or failure */
> u64 paddr;
> };
>
> Driver must send the reply via the request queue, with the fault status
> in 'reply', and the mapped page in 'paddr' on success.
>
> Existing fault handling interfaces such as PRI have a tag (PRG) allowing
> to identify a page request (or group thereof) when sending a reply. I
> wonder if this would be useful to us, but it seems like the
> (address_space, context, vaddr) tuple is sufficient to identify a page
> fault, provided the device doesn't send duplicate faults. Duplicate faults
> could be required if they have a side effect, for instance implementing a
> poor man's doorbell. If this is desirable, we could add a fault_id field.
>
>
> 4. Host implementation with VFIO
> --------------------------------
>
> The VFIO interface for sharing page tables is being worked on at the
> moment by Intel. Other virtual IOMMU implementation will most likely let
> guest manage full context tables (PASID tables) themselves, giving the
> context table pointer to the pIOMMU via a VFIO ioctl.
>
> For the architecture-agnostic virtio-iommu however, we shouldn't have to
> implement all possible formats of context table (they are at least
> different between ARM SMMU and Intel IOMMU, and will certainly be extended
> in future physical IOMMU architectures.) In addition, most users might
> only care about having one page directory per device, as SVM is a luxury
> at the moment and few devices support it. For these reasons, we should
> allow to pass single page directories via VFIO, using very similar
> structures as described above, whilst reusing the VFIO channel developed
> for Intel vIOMMU.
>
> * VFIO_SVM_INFO: probe page table formats
> * VFIO_SVM_BIND: set pgd and arch-specific configuration
>
> There is an inconvenient with letting the pIOMMU driver manage the guest's
> context table. During a page table walk, the pIOMMU translates the context
> table pointer using the stage-2 page tables. The context table must
> therefore be mapped in guest-physical space by the pIOMMU driver. One
> solution is to let the pIOMMU driver reserve some GPA space upfront using
> the iommu and sysfs resv API [1]. The host would then carve that region
> out of the guest-physical space using a firmware mechanism (for example DT
> reserved-memory node).
>
>
> III. Relaxed operations
> =======================
>
> VIRTIO_IOMMU_F_RELAXED
>
> Adding an IOMMU dramatically reduces performance of a device, because
> map/unmap operations are costly and produce a lot of TLB traffic. For
> significant performance improvements, device might allow the driver to
> sacrifice safety for speed. In this mode, the driver does not need to send
> UNMAP requests. The semantics of MAP change and are more complex to
> implement. Given a MAP([start:end] -> phys, flags) request:
>
> (1) If [start:end] isn't mapped, request succeeds as usual.
> (2) If [start:end] overlaps an existing mapping [old_start:old_end], we
> unmap [max(start, old_start):min(end, old_end)] and replace it with
> [start:end].
> (3) If [start:end] overlaps an existing mapping that matches the new map
> request exactly (same flags, same phys address), the old mapping is
> kept.
>
> This squashing could be performed by the guest. The driver can catch unmap
> requests from the DMA layer, and only relay map requests for (1) and (2).
> A MAP request is therefore able to split and partially override an
> existing mapping, which isn't allowed in non-relaxed mode. UNMAP requests
> are unnecessary, but are now allowed to split or carve holes in mappings.
>
> In this model, a MAP request may take longer, but we may have a net gain
> by removing a lot of redundant requests. Squashing series of map/unmap
> performed by the guest for the same mapping improves temporal reuse of
> IOVA mappings, which I can observe by simply dumping IOMMU activity of a
> virtio device. It reduce the number of TLB invalidations to the strict
> minimum while keeping correctness of DMA operations (provided the device
> obeys its driver). There is a good read on the subject of optimistic
> teardown in paper [2].
>
> This model is completely unsafe. A stale DMA transaction might access a
> page long after the device driver in the guest unmapped it and
> decommissioned the page. The DMA transaction might hit into a completely
> different part of the system that is now reusing the page. Existing
> relaxed implementations attempt to mitigate the risk by setting a timeout
> on the teardown. Unmap requests from device drivers are not discarded
> entirely, but buffered and sent at a later time. Paper [2] reports good
> results with a 10ms delay.
>
> We could add a way for device and driver to negotiate a vulnerability
> window to mitigate the risk of DMA attacks. Driver might not accept a
> window at all, since it requires more infrastructure to keep delayed
> mappings. In my opinion, it should be made clear that regardless of the
> duration of this window, any driver accepting F_RELAXED feature makes the
> guest completely vulnerable, and the choice boils down to either isolation
> or speed, not a bit of both.
>
>
> IV. Misc
> ========
>
> I think we have enough to go on for a while. To improve MAP throughput, I
> considered adding a MAP_SG request depending on a feature bit, with
> variable size:
>
> struct virtio_iommu_req_map_sg {
> struct virtio_iommu_req_head;
> u32 address_space;
> u32 nr_elems;
> u64 virt_addr;
> u64 size;
> u64 phys_addr[nr_elems];
> };
>
> Would create the following mappings:
>
> virt_addr -> phys_addr[0]
> virt_addr + size -> phys_addr[1]
> virt_addr + 2 * size -> phys_addr[2]
> ...
>
> This would avoid the overhead of multiple map commands. We could try to
> find a more cunning format to compress virtually-contiguous mappings with
> different (phys, size) pairs as well. But Linux drivers rarely prefer
> map_sg() functions over regular map(), so I don't know if the whole map_sg
> feature is worth the effort. All we would gain is a few bytes anyway.
>
> My current map_sg implementation in the virtio-iommu driver adds a batch
> of map requests to the queue and kick the host once. That might be enough
> of an optimization.
>
>
> Another invasive optimization would be adding grouped requests. By adding
> two flags in the header, L and G, we can group sequences of requests
> together, and have one status at the end, either 0 if all requests in the
> group succeeded, or the status of the first request that failed. This is
> all in-order. Requests in a group follow each others, there is no sequence
> identifier.
>
> ___ L: request is last in the group
> / _ G: request is part of a group
> | /
> v v
> 31 9 8 7 0
> +--------------------------------+ <------- RO descriptor
> | res0 |0|1| type |
> +--------------------------------+
> | payload |
> +--------------------------------+
> | res0 |0|1| type |
> +--------------------------------+
> | payload |
> +--------------------------------+
> | res0 |0|1| type |
> +--------------------------------+
> | payload |
> +--------------------------------+
> | res0 |1|1| type |
> +--------------------------------+
> | payload |
> +--------------------------------+ <------- WO descriptor
> | res0 | status |
> +--------------------------------+
>
> This adds some complexity on the device, since it must unroll whatever was
> done by successful requests in a group as soon as one fails, and reject
> all subsequent ones. A group of requests is an atomic operation. As with
> map_sg, this change mostly allows to save space and virtio descriptors.
>
>
> [1] https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> [2] vIOMMU: Efficient IOMMU Emulation
> N. Amit, M. Ben-Yehuda, D. Tsafrir, A. Schuster
^ permalink raw reply
* RE: [virtio-dev] Re: [PATCH v9 2/5] virtio-balloon: VIRTIO_BALLOON_F_BALLOON_CHUNKS
From: Wang, Wei W @ 2017-04-26 11:03 UTC (permalink / raw)
To: Wang, Wei W, Michael S. Tsirkin
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, Hansen, Dave,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <58F43801.7060004@intel.com>
Hi Michael, could you please give some feedback?
On Monday, April 17, 2017 11:35 AM, Wei Wang wrote:
> On 04/15/2017 05:38 AM, Michael S. Tsirkin wrote:
> > On Fri, Apr 14, 2017 at 04:37:52PM +0800, Wei Wang wrote:
> >> On 04/14/2017 12:34 AM, Michael S. Tsirkin wrote:
> >>> On Thu, Apr 13, 2017 at 05:35:05PM +0800, Wei Wang wrote:
> >>>
> >>> So we don't need the bitmap to talk to host, it is just a data
> >>> structure we chose to maintain lists of pages, right?
> >> Right. bitmap is the way to gather pages to chunk.
> >> It's only needed in the balloon page case.
> >> For the unused page case, we don't need it, since the free page
> >> blocks are already chunks.
> >>
> >>> OK as far as it goes but you need much better isolation for it.
> >>> Build a data structure with APIs such as _init, _cleanup, _add,
> >>> _clear, _find_first, _find_next.
> >>> Completely unrelated to pages, it just maintains bits.
> >>> Then use it here.
> >>>
> >>>
> >>>> static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
> >>>> module_param(oom_pages, int, S_IRUSR | S_IWUSR);
> >>>> MODULE_PARM_DESC(oom_pages, "pages to free on OOM"); @@ -50,6
> >>>> +54,10 @@ MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
> >>>> static struct vfsmount *balloon_mnt;
> >>>> #endif
> >>>> +/* Types of pages to chunk */
> >>>> +#define PAGE_CHUNK_TYPE_BALLOON 0
> >>>> +
> >>> Doesn't look like you are ever adding more types in this patchset.
> >>> Pls keep code simple, generalize it later.
> >>>
> >> "#define PAGE_CHUNK_TYPE_UNUSED 1" is added in another patch.
> > I would say add the extra code there too. Or maybe we can avoid adding
> > it altogether.
>
> I'm trying to have the two features( i.e. "balloon pages" and "unused pages")
> decoupled while trying to use common functions to deal with the commonalities.
> That's the reason to define the above macro.
> Without the macro, we will need to have separate functions, for example,
> instead of one "add_one_chunk()", we need to have
> add_one_balloon_page_chunk() and add_one_unused_page_chunk(), and some
> of the implementations will be kind of duplicate in the two functions.
> Probably we can add it when the second feature comes to the code.
>
> >
> >> Types of page to chunk are treated differently. Different types of
> >> page chunks are sent to the host via different protocols.
> >>
> >> 1) PAGE_CHUNK_TYPE_BALLOON: Ballooned (i.e. inflated/deflated) pages
> >> to chunk. For the ballooned type, it uses the basic chunk msg format:
> >>
> >> virtio_balloon_page_chunk_hdr +
> >> virtio_balloon_page_chunk * MAX_PAGE_CHUNKS
> >>
> >> 2) PAGE_CHUNK_TYPE_UNUSED: unused pages to chunk. It uses this miscq
> >> msg
> >> format:
> >> miscq_hdr +
> >> virtio_balloon_page_chunk_hdr +
> >> virtio_balloon_page_chunk * MAX_PAGE_CHUNKS
> >>
> >> The chunk msg is actually the payload of the miscq msg.
> >>
> >>
> > So just combine the two message formats and then it'll all be easier?
> >
>
> Yes, it'll be simple with only one msg format. But the problem I see here is that
> miscq hdr is something necessary for the "unused page"
> usage, but not needed by the "balloon page" usage. To be more precise, struct
> virtio_balloon_miscq_hdr {
> __le16 cmd;
> __le16 flags;
> };
> 'cmd' specifies the command from the miscq (I envision that miscq will be
> further used to handle other possible miscellaneous requests either from the
> host or to the host), so 'cmd' is necessary for the miscq. But the inflateq is
> exclusively used for inflating pages, so adding a command to it would be
> redundant and look a little bewildered there.
> 'flags': We currently use bit 0 of flags to indicate the completion ofa command,
> this is also useful in the "unused page" usage, and not needed by the "balloon
> page" usage.
> >>>> +#define MAX_PAGE_CHUNKS 4096
> >>> This is an order-4 allocation. I'd make it 4095 and then it's an
> >>> order-3 one.
> >> Sounds good, thanks.
> >> I think it would be better to make it 4090. Leave some space for the
> >> hdr as well.
> > And miscq hdr. In fact just let compiler do the math - something like:
> > (8 * PAGE_SIZE - sizeof(hdr)) / sizeof(chunk)
> Agree, thanks.
>
> >
> > I skimmed explanation of algorithms below but please make sure code
> > speaks for itself and add comments inline to document it.
> > Whenever you answered me inline this is where you want to try to make
> > code clearer and add comments.
> >
> > Also, pls find ways to abstract the data structure so we don't need to
> > deal with its internals all over the code.
> >
> >
> > ....
> >
> >>>> {
> >>>> struct scatterlist sg;
> >>>> + struct virtio_balloon_page_chunk_hdr *hdr;
> >>>> + void *buf;
> >>>> unsigned int len;
> >>>> - sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
> >>>> + switch (type) {
> >>>> + case PAGE_CHUNK_TYPE_BALLOON:
> >>>> + hdr = vb->balloon_page_chunk_hdr;
> >>>> + len = 0;
> >>>> + break;
> >>>> + default:
> >>>> + dev_warn(&vb->vdev->dev, "%s: chunk %d of unknown
> pages\n",
> >>>> + __func__, type);
> >>>> + return;
> >>>> + }
> >>>> - /* We should always be able to add one buffer to an empty queue. */
> >>>> - virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> >>>> - virtqueue_kick(vq);
> >>>> + buf = (void *)hdr - len;
> >>> Moving back to before the header? How can this make sense?
> >>> It works fine since len is 0, so just buf = hdr.
> >>>
> >> For the unused page chunk case, it follows its own protocol:
> >> miscq_hdr + payload(chunk msg).
> >> "buf = (void *)hdr - len" moves the buf pointer to the miscq_hdr,
> >> to send the entire miscq msg.
> > Well just pass the correct pointer in.
> >
> OK. The miscq msg is
> {
> miscq_hdr;
> chunk_msg;
> }
>
> We can probably change the code like this:
>
> #define CHUNK_TO_MISCQ_MSG(chunk) (chunk - sizeof(struct
> virtio_balloon_miscq_hdr))
>
> switch (type) {
> case PAGE_CHUNK_TYPE_BALLOON:
> msg_buf = vb->balloon_page_chunk_hdr;
> msg_len = sizeof(struct virtio_balloon_page_chunk_hdr) +
> nr_chunks * sizeof(struct virtio_balloon_page_chunk_entry);
> break;
> case PAGE_CHUNK_TYPE_UNUSED:
> msg_buf = CHUNK_TO_MISCQ_MSG(vb->unused_page_chunk_hdr);
> msg_len = sizeof(struct virtio_balloon_miscq_hdr) + sizeof(struct
> virtio_balloon_page_chunk_hdr) +
> nr_chunks * sizeof(struct virtio_balloon_page_chunk_entry);
> break;
> default:
> dev_warn(&vb->vdev->dev, "%s: chunk %d of unknown pages\n",
> __func__, type);
> return;
> }
>
>
>
> >> Please check the patch for implementing the unused page chunk, it
> >> will be clear. If necessary, I can put "buf = (void *)hdr - len" from
> >> that patch.
> > Exactly. And all this pointer math is very messy. Please look for ways
> > to clean it. It's generally easy to fill structures:
> >
> > struct foo *foo = kmalloc(..., sizeof(*foo) + n * sizeof(foo->a[0]));
> > for (i = 0; i < n; ++i)
> > foo->a[i] = b;
> >
> > this is the kind of code that's easy to understand and it's obvious
> > there are no overflows and no info leaks here.
> >
> OK, will take your suggestion:
>
> struct virtio_balloon_page_chunk {
> struct virtio_balloon_page_chunk_hdr hdr;
> struct virtio_balloon_page_chunk_entry entries[]; };
>
>
> >>>> + len += sizeof(struct virtio_balloon_page_chunk_hdr);
> >>>> + len += hdr->chunks * sizeof(struct virtio_balloon_page_chunk);
> >>>> + sg_init_table(&sg, 1);
> >>>> + sg_set_buf(&sg, buf, len);
> >>>> + if (!virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL)) {
> >>>> + virtqueue_kick(vq);
> >>>> + if (busy_wait)
> >>>> + while (!virtqueue_get_buf(vq, &len) &&
> >>>> + !virtqueue_is_broken(vq))
> >>>> + cpu_relax();
> >>>> + else
> >>>> + wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> >>>> + hdr->chunks = 0;
> >>> Why zero it here after device used it? Better to zero before use.
> >> hdr->chunks tells the host how many chunks are there in the payload.
> >> After the device use it, it is ready to zero it.
> > It's rather confusing. Try to pass # of chunks around in some other
> > way.
>
> Not sure if this was explained clearly - we just let the chunk msg hdr indicates
> the # of chunks in the payload. I think this should be a pretty normal usage, like
> the network UDP hdr, which uses a length field to indicate the packet length.
>
> >>>> + }
> >>>> +}
> >>>> +
> >>>> +static void add_one_chunk(struct virtio_balloon *vb, struct virtqueue *vq,
> >>>> + int type, u64 base, u64 size)
> >>> what are the units here? Looks like it's in 4kbyte units?
> >> what is the "unit" you referred to?
> >> This is the function to add one chunk, base pfn and size of the chunk
> >> are supplied to the function.
> >>
> > Are both size and base in bytes then?
> > But you do not send them to host as is, you shift them for some reason
> > before sending them to host.
> >
> Not in bytes actually. base is a base pfn, which is the starting address of the
> continuous pfns. Size is the chunk size, which is the number of continuous pfns.
>
> They are shifted based on the chunk format we agreed before:
>
> --------------------------------------------------------
> | Base (52 bit) | Rsvd (12 bit) |
> --------------------------------------------------------
> --------------------------------------------------------
> | Size (52 bit) | Rsvd (12 bit) |
> --------------------------------------------------------
>
>
> Here, the pfn will be the balloon page pfn (4KB).In this way, the host doesn't
> need to know PAGE_SIZE of the guest.
>
>
>
> >>>> + if (zero >= end)
> >>>> + chunk_size = end - one;
> >>>> + else
> >>>> + chunk_size = zero - one;
> >>>> +
> >>>> + if (chunk_size)
> >>>> + add_one_chunk(vb, vq,
> PAGE_CHUNK_TYPE_BALLOON,
> >>>> + pfn_start + one, chunk_size);
> >>> Still not so what does a bit refer to? page or 4kbytes?
> >>> I think it should be a page.
> >> A bit in the bitmap corresponds to a pfn of a balloon page(4KB).
> > That's a waste on systems with large page sizes, and it does not look
> > like you handle that case correctly.
>
> OK, I will change the bitmap to be PAGE_SIZE based here, instead of
> BALLOON_PAGE_SIZE based. When convert them into chunks, making it based
> on BALLOON_PAGE_SIZE.
>
>
> Best,
> Wei
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox