* [PATCH 0/6] jailhouse: Enhance secondary Jailhouse guest support /wrt PCI
From: Jan Kiszka @ 2018-01-22 6:12 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Bjorn Helgaas
Cc: jailhouse-dev, Mark Rutland, Benedikt Spranger, linux-pci, x86,
Linux Kernel Mailing List, virtualization, Rob Herring,
Otavio Pontes
Basic x86 support [1] for running Linux as secondary Jailhouse [2] guest
is currently pending in the tip tree. This builds on top and enhances
the PCI support for x86 and also ARM guests (ARM[64] does not require
platform patches and works already).
Key elements of this series are:
- detection of Jailhouse via device tree hypervisor node
- function-level PCI scan if Jailhouse is detected
- MMCONFIG support for x86 guests
As most changes affect x86, I would suggest to route the series also via
tip after the necessary acks are collected.
Jan
[1] https://lkml.org/lkml/2017/11/27/125
[2] http://jailhouse-project.org
CC: Benedikt Spranger <b.spranger@linutronix.de>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Otavio Pontes <otavio.pontes@intel.com>
CC: Rob Herring <robh+dt@kernel.org>
Jan Kiszka (5):
jailhouse: Provide detection for non-x86 systems
pci: Scan all functions when probing while running over Jailhouse
x86: Consolidate PCI_MMCONFIG configs
x86/jailhouse: Allow to use PCI_MMCONFIG without ACPI
MAINTAINERS: Add entry for Jailhouse
Otavio Pontes (1):
x86/jailhouse: Enable PCI mmconfig access in inmates
Documentation/devicetree/bindings/jailhouse.txt | 8 ++++++++
MAINTAINERS | 7 +++++++
arch/x86/Kconfig | 11 ++++++-----
arch/x86/include/asm/jailhouse_para.h | 2 +-
arch/x86/include/asm/pci_x86.h | 2 ++
arch/x86/kernel/Makefile | 2 +-
arch/x86/kernel/cpu/amd.c | 2 +-
arch/x86/kernel/jailhouse.c | 7 +++++++
arch/x86/pci/legacy.c | 4 +++-
arch/x86/pci/mmconfig-shared.c | 4 ++--
drivers/pci/probe.c | 4 +++-
include/linux/hypervisor.h | 17 +++++++++++++++--
12 files changed, 56 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/jailhouse.txt
--
2.13.6
^ permalink raw reply
* Re: [RFC PATCH v2 2/5] iommu/virtio-iommu: Add probe request
From: Auger Eric @ 2018-01-19 17:22 UTC (permalink / raw)
To: Jean-Philippe Brucker, iommu@lists.linux-foundation.org,
devel@acpica.org, linux-acpi@vger.kernel.org, kvm@vger.kernel.org,
kvmarm@lists.cs.columbia.edu,
virtualization@lists.linux-foundation.org,
virtio-dev@lists.oasis-open.org
Cc: Jayachandran.Nair@cavium.com, Lorenzo Pieralisi,
ashok.raj@intel.com, mst@redhat.com, Marc Zyngier, Will Deacon,
rjw@rjwysocki.net, robert.moore@intel.com, lv.zheng@intel.com,
Sudeep Holla, lenb@kernel.org, Robin Murphy, joro@8bytes.org,
hanjun.guo@linaro.org
In-Reply-To: <b9ede9e9-25a6-2848-e152-ffc61d560be8@arm.com>
Hi Jean-Philippe,
On 19/01/18 17:21, Jean-Philippe Brucker wrote:
> On 16/01/18 23:26, Auger Eric wrote:
> [...]
>>> + switch (mem->subtype) {
>>> + case VIRTIO_IOMMU_RESV_MEM_T_MSI:
>>> + region = iommu_alloc_resv_region(addr, size, prot,
>>> + IOMMU_RESV_MSI);
>> if (!region)
>> return -ENOMEM;
>>> + break;
>>> + case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
>>> + default:
>>> + region = iommu_alloc_resv_region(addr, size, 0,
>>> + IOMMU_RESV_RESERVED);
>> same.
>
> I'll add them, thanks
>
>> There is another issue related to the exclusion of iovas belonging to
>> reserved regions. Typically on x86, when attempting to run
>> virtio-blk-pci with iommu I eventually saw the driver using iova
>> belonging to the IOAPIC regions to map phys addr and this stalled qemu
>> with a drown trace:
>>
>> "virtio: bogus descriptor or out of resources"
>>
>> those regions need to be excluded from the iova allocator. This was
>> resolved by adding
>> if (iommu_dma_init_domain(domain,
>> vdev->viommu->geometry.aperture_start,
>> vdev->viommu->geometry.aperture_end,
>> dev))
>> in viommu_attach_dev()
>
> The most recent hack for x86 [1] does call iommu_dma_init_domain() in
> attach_dev(). Is it buggy?
Oh OK. Actually I have never used that branch and in my version the last
arg of iommu_dma_init_domain was NULL hence the issue. But I was
thinking more generally that care must be taken to exclude all those
regions.
Thanks
Eric
>
> We probably shouldn't call iommu_dma_init_domain() unconditionally
> (outside of CONFIG_X86 that is), since it's normally done by the arch
> (arch/arm64/mm/dma-mapping.c)
>
> Thanks,
> Jean
>
> [1]
> http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=e910e224b58712151dda06df595a53ff07edef63
> on branch virtio-iommu/v0.5-x86
>
^ permalink raw reply
* Re: [PATCH] virtio_blk: print capacity at probe time
From: Stefan Hajnoczi @ 2018-01-19 16:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Hajnoczi, linux-kernel, Rodrigo A B Freire,
Linux Virtualization
In-Reply-To: <20180103160339.21300-1-stefanha@redhat.com>
On Wed, Jan 3, 2018 at 4:03 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Print the capacity of the block device when the driver is probed. Many
> users expect this since SCSI disks (sd) do it. Moreover, kernel dmesg
> output is the primary source of troubleshooting information so it's
> helpful to include the disk size there.
>
> The capacity is already printed by virtio_blk when a resize event
> occurs. Extract the code and reuse it from virtblk_probe().
>
> This patch also adds the block device name to the message so it can be
> correlated with a specific device:
>
> virtio_blk virtio0: [vda] 20971520 512-byte logical blocks (10.7 GB/10.0 GiB)
>
> Cc: Rodrigo A B Freire <rfreire@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> drivers/block/virtio_blk.c | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
Ping?
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 68846897d213..787cd2a10b0b 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -373,14 +373,12 @@ static ssize_t virtblk_serial_show(struct device *dev,
>
> static DEVICE_ATTR(serial, S_IRUGO, virtblk_serial_show, NULL);
>
> -static void virtblk_config_changed_work(struct work_struct *work)
> +/* The queue's logical block size must be set before calling this */
> +static void virtblk_update_capacity(struct virtio_blk *vblk, bool resize)
> {
> - struct virtio_blk *vblk =
> - container_of(work, struct virtio_blk, config_work);
> struct virtio_device *vdev = vblk->vdev;
> struct request_queue *q = vblk->disk->queue;
> char cap_str_2[10], cap_str_10[10];
> - char *envp[] = { "RESIZE=1", NULL };
> unsigned long long nblocks;
> u64 capacity;
>
> @@ -402,13 +400,24 @@ static void virtblk_config_changed_work(struct work_struct *work)
> STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
>
> dev_notice(&vdev->dev,
> - "new size: %llu %d-byte logical blocks (%s/%s)\n",
> + "[%s] %s%llu %d-byte logical blocks (%s/%s)\n",
> + vblk->disk->disk_name,
> + resize ? "new size: " : "",
> nblocks,
> queue_logical_block_size(q),
> cap_str_10,
> cap_str_2);
>
> set_capacity(vblk->disk, capacity);
> +}
> +
> +static void virtblk_config_changed_work(struct work_struct *work)
> +{
> + struct virtio_blk *vblk =
> + container_of(work, struct virtio_blk, config_work);
> + char *envp[] = { "RESIZE=1", NULL };
> +
> + virtblk_update_capacity(vblk, true);
> revalidate_disk(vblk->disk);
> kobject_uevent_env(&disk_to_dev(vblk->disk)->kobj, KOBJ_CHANGE, envp);
> }
> @@ -621,7 +630,6 @@ static int virtblk_probe(struct virtio_device *vdev)
> struct request_queue *q;
> int err, index;
>
> - u64 cap;
> u32 v, blk_size, sg_elems, opt_io_size;
> u16 min_io_size;
> u8 physical_block_exp, alignment_offset;
> @@ -719,17 +727,6 @@ static int virtblk_probe(struct virtio_device *vdev)
> if (virtio_has_feature(vdev, VIRTIO_BLK_F_RO))
> set_disk_ro(vblk->disk, 1);
>
> - /* Host must always specify the capacity. */
> - virtio_cread(vdev, struct virtio_blk_config, capacity, &cap);
> -
> - /* If capacity is too big, truncate with warning. */
> - if ((sector_t)cap != cap) {
> - dev_warn(&vdev->dev, "Capacity %llu too large: truncating\n",
> - (unsigned long long)cap);
> - cap = (sector_t)-1;
> - }
> - set_capacity(vblk->disk, cap);
> -
> /* We can handle whatever the host told us to handle. */
> blk_queue_max_segments(q, vblk->sg_elems-2);
>
> @@ -780,6 +777,7 @@ static int virtblk_probe(struct virtio_device *vdev)
> if (!err && opt_io_size)
> blk_queue_io_opt(q, blk_size * opt_io_size);
>
> + virtblk_update_capacity(vblk, false);
> virtio_device_ready(vdev);
>
> device_add_disk(&vdev->dev, vblk->disk);
> --
> 2.14.3
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC PATCH v2 2/5] iommu/virtio-iommu: Add probe request
From: Jean-Philippe Brucker @ 2018-01-19 16:21 UTC (permalink / raw)
To: Auger Eric, iommu@lists.linux-foundation.org, devel@acpica.org,
linux-acpi@vger.kernel.org, kvm@vger.kernel.org,
kvmarm@lists.cs.columbia.edu,
virtualization@lists.linux-foundation.org,
virtio-dev@lists.oasis-open.org
Cc: Jayachandran.Nair@cavium.com, Lorenzo Pieralisi,
ashok.raj@intel.com, mst@redhat.com, Marc Zyngier, Will Deacon,
rjw@rjwysocki.net, robert.moore@intel.com, lv.zheng@intel.com,
Sudeep Holla, lenb@kernel.org, Robin Murphy, joro@8bytes.org,
hanjun.guo@linaro.org
In-Reply-To: <5b236aaa-4abb-2cac-1c74-f8137a0c2244@redhat.com>
On 16/01/18 23:26, Auger Eric wrote:
[...]
>> + switch (mem->subtype) {
>> + case VIRTIO_IOMMU_RESV_MEM_T_MSI:
>> + region = iommu_alloc_resv_region(addr, size, prot,
>> + IOMMU_RESV_MSI);
> if (!region)
> return -ENOMEM;
>> + break;
>> + case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
>> + default:
>> + region = iommu_alloc_resv_region(addr, size, 0,
>> + IOMMU_RESV_RESERVED);
> same.
I'll add them, thanks
> There is another issue related to the exclusion of iovas belonging to
> reserved regions. Typically on x86, when attempting to run
> virtio-blk-pci with iommu I eventually saw the driver using iova
> belonging to the IOAPIC regions to map phys addr and this stalled qemu
> with a drown trace:
>
> "virtio: bogus descriptor or out of resources"
>
> those regions need to be excluded from the iova allocator. This was
> resolved by adding
> if (iommu_dma_init_domain(domain,
> vdev->viommu->geometry.aperture_start,
> vdev->viommu->geometry.aperture_end,
> dev))
> in viommu_attach_dev()
The most recent hack for x86 [1] does call iommu_dma_init_domain() in
attach_dev(). Is it buggy?
We probably shouldn't call iommu_dma_init_domain() unconditionally
(outside of CONFIG_X86 that is), since it's normally done by the arch
(arch/arm64/mm/dma-mapping.c)
Thanks,
Jean
[1]
http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=e910e224b58712151dda06df595a53ff07edef63
on branch virtio-iommu/v0.5-x86
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Michael S. Tsirkin @ 2018-01-19 12:39 UTC (permalink / raw)
To: Wei Wang
Cc: yang.zhang.wz, virtio-dev, riel, quan.xu0, kvm, nilal,
liliang.opensource, linux-kernel, mhocko, linux-mm, pbonzini,
akpm, virtualization
In-Reply-To: <5A616995.4050702@intel.com>
On Fri, Jan 19, 2018 at 11:44:21AM +0800, Wei Wang wrote:
> On 01/18/2018 12:44 AM, Michael S. Tsirkin wrote:
> > On Wed, Jan 17, 2018 at 01:10:11PM +0800, Wei Wang wrote:
> >
>
> >
> > > +{
> > > + struct scatterlist sg;
> > > + unsigned int unused;
> > > + int err;
> > > +
> > > + sg_init_one(&sg, addr, sizeof(uint32_t));
> > This passes a guest-endian value to host. This is a problem:
> > should always pass LE values.
>
> I think the endianness is handled when virtqueue_add_outbuf():
>
> desc[i].addr = cpu_to_virtio64(_vq->vdev, addr);
>
> right?
No - that handles the address, not the value you pass in.
> >
> > > +
> > > + /*
> > > + * This handles the cornercase that the vq happens to be full when
> > > + * adding a cmd id. Rarely happen in practice.
> > > + */
> > > + while (!vq->num_free)
> > > + virtqueue_get_buf(vq, &unused);
> > I dislike this busy-waiting. It's a hint after all -
> > why not just retry later - hopefully after getting an
> > interrupt?
> >
> > Alternatively, stop adding more entries when we have a single
> > ring entry left, making sure we have space for the command.
>
> I think the second one looks good. Thanks.
>
> > > + queue_work(system_freezable_wq,
> > > + &vb->update_balloon_size_work);
> > > + spin_unlock_irqrestore(&vb->stop_update_lock, flags);
> > > + }
> > > +
> > > + virtio_cread(vb->vdev, struct virtio_balloon_config,
> > > + free_page_report_cmd_id, &cmd_id);
> > You want virtio_cread_feature, don't access the new field
> > if the feature has not been negotiated.
>
> Right. We probably need to put all the following cmd id related things under
> the feature check,
>
> How about
>
> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
> virtio_cread(..);
> if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
> ....
> }
>
that's ok too.
> >
> >
> > > + if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
> > > + WRITE_ONCE(vb->report_free_page, false);
> > > + } else if (cmd_id != vb->start_cmd_id) {
> > > + /*
> > > + * Host requests to start the reporting by sending a new cmd
> > > + * id.
> > > + */
> > > + WRITE_ONCE(vb->report_free_page, true);
> > I don't know why we bother with WRITE_ONCE here. The point of
> > report_free_page being used lockless is that that it's not a big deal if
> > it's wrong occasionally, right?
>
> Actually the main reason is that "vb->report_free_page" is a value shared by
> two threads:
> Written by the config_change here, and read by the worker thread that
> reports the free pages.
Right but what's wrong if it's read or written twice and not once?
> Alternatively, we could let the two sides access to the shared variable with
> "volatile" pointers.
>
>
> >
> >
> >
> > > + vb->start_cmd_id = cmd_id;
> > > + queue_work(vb->balloon_wq, &vb->report_free_page_work);
> > It seems that if a command was already queued (with a different id),
> > this will result in new command id being sent to host twice, which will
> > likely confuse the host.
>
> I think that case won't happen, because
> - the host sends a cmd id to the guest via the config, while the guest acks
> back the received cmd id via the virtqueue;
> - the guest ack back a cmd id only when a new cmd id is received from the
> host, that is the above check:
>
> if (cmd_id != vb->start_cmd_id) { --> the driver only queues the
> reporting work only when a new cmd id is received
> /*
> * Host requests to start the reporting by sending a
> * new cmd id.
> */
> WRITE_ONCE(vb->report_free_page, true);
> vb->start_cmd_id = cmd_id;
> queue_work(vb->balloon_wq,
> &vb->report_free_page_work);
> }
>
> So the same cmd id wouldn't queue the reporting work twice.
>
Like this:
vb->start_cmd_id = cmd_id;
queue_work(vb->balloon_wq, &vb->report_free_page_work);
command id changes
vb->start_cmd_id = cmd_id;
work executes
queue_work(vb->balloon_wq, &vb->report_free_page_work);
work executes again
> >
> >
> >
> > > + }
> > > +}
> > > +
> > > static void update_balloon_size(struct virtio_balloon *vb)
> > > {
> > > u32 actual = vb->num_pages;
> > > @@ -417,40 +513,113 @@ static void update_balloon_size_func(struct work_struct *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;
> > > + struct scatterlist sg;
> > > + int i, nvqs, err = -ENOMEM;
> > > +
> > > + /* 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_FREE_PAGE_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;
> > Why not just keep these 3 arrays on stack? they aren't large.
>
> Sounds good. Here is the new implementation:
>
> static int init_vqs(struct virtio_balloon *vb)
> {
> struct virtqueue *vqs[4];
> vq_callback_t *callbacks[4];
> const char *names[4];
> struct scatterlist sg;
> int ret;
>
>
> /*
> * Inflateq and deflateq are used unconditionally. stats_vq and
> * free_page_vq uses names[2] and names[3], respectively. The
> names[]
> * will be NULL if the related feature is not enabled, which will
> * cause no allocation for the corresponding virtqueue in find_vqs.
> */
> callbacks[0] = balloon_ack;
> names[0] = "inflate";
> callbacks[1] = balloon_ack;
> names[1] = "deflate";
> names[2] = NULL;
> names[3] = NULL;
>
> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
> names[2] = "stats";
> callbacks[2] = stats_request;
> }
> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
> names[3] = "free_page_vq";
> callbacks[3] = NULL;
> }
>
> ret = vb->vdev->config->find_vqs(vb->vdev, 4, vqs, callbacks, names,
> NULL, NULL);
> if (ret)
> return ret;
>
> vb->inflate_vq = vqs[0];
> vb->deflate_vq = vqs[1];
>
> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
> vb->stats_vq = vqs[2];
> /*
> * Prime this virtqueue with one buffer so the hypervisor
> can
> * use it to signal us later (it can't be broken yet!).
> */
> sg_init_one(&sg, vb->stats, sizeof(vb->stats));
> ret = virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb,
> GFP_KERNEL);
> if (ret) {
> dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n",
> __func__);
> return ret;
> }
> virtqueue_kick(vb->stats_vq);
> }
>
> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ))
> vb->free_page_vq = vqs[3];
>
> return 0;
> }
>
>
> Btw, the QEMU side doesn't have an option to disable STATS_VQ currently, we
> may need to add that later.
>
> Best,
> Wei
why not
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Wei Wang @ 2018-01-19 6:24 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: yang.zhang.wz, virtio-dev, riel, quan.xu0, kvm, nilal,
liliang.opensource, linux-kernel, mhocko, linux-mm, pbonzini,
akpm, virtualization
In-Reply-To: <20180117180337-mutt-send-email-mst@kernel.org>
On 01/18/2018 12:44 AM, Michael S. Tsirkin wrote:
> On Wed, Jan 17, 2018 at 01:10:11PM +0800, Wei Wang wrote:
>>
>> +static void virtballoon_changed(struct virtio_device *vdev)
>> +{
>> + struct virtio_balloon *vb = vdev->priv;
>> + unsigned long flags;
>> + __u32 cmd_id;
>> + s64 diff = towards_target(vb);
>> +
>> + if (diff) {
>> + spin_lock_irqsave(&vb->stop_update_lock, flags);
>> + if (!vb->stop_update)
> Why do you ignore stop_update for freeze?
> This means new wq entries can be added during remove
> causing use after free issues.
I think stop_update isn't needed, because the lock has already been
handled internally by the APIs. Similar examples like
mem_cgroup_css_free() in "mm/memcontrol.c", there is no such locks used
for cancel_work_sync(&memcg->high_work).
Best,
Wei
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Wei Wang @ 2018-01-19 3:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: yang.zhang.wz, virtio-dev, riel, quan.xu0, kvm, nilal,
liliang.opensource, linux-kernel, mhocko, linux-mm, pbonzini,
akpm, virtualization
In-Reply-To: <20180117180337-mutt-send-email-mst@kernel.org>
On 01/18/2018 12:44 AM, Michael S. Tsirkin wrote:
> On Wed, Jan 17, 2018 at 01:10:11PM +0800, Wei Wang wrote:
>
>
>> +{
>> + struct scatterlist sg;
>> + unsigned int unused;
>> + int err;
>> +
>> + sg_init_one(&sg, addr, sizeof(uint32_t));
> This passes a guest-endian value to host. This is a problem:
> should always pass LE values.
I think the endianness is handled when virtqueue_add_outbuf():
desc[i].addr = cpu_to_virtio64(_vq->vdev, addr);
right?
>
>> +
>> + /*
>> + * This handles the cornercase that the vq happens to be full when
>> + * adding a cmd id. Rarely happen in practice.
>> + */
>> + while (!vq->num_free)
>> + virtqueue_get_buf(vq, &unused);
> I dislike this busy-waiting. It's a hint after all -
> why not just retry later - hopefully after getting an
> interrupt?
>
> Alternatively, stop adding more entries when we have a single
> ring entry left, making sure we have space for the command.
I think the second one looks good. Thanks.
>> + queue_work(system_freezable_wq,
>> + &vb->update_balloon_size_work);
>> + spin_unlock_irqrestore(&vb->stop_update_lock, flags);
>> + }
>> +
>> + virtio_cread(vb->vdev, struct virtio_balloon_config,
>> + free_page_report_cmd_id, &cmd_id);
> You want virtio_cread_feature, don't access the new field
> if the feature has not been negotiated.
Right. We probably need to put all the following cmd id related things
under the feature check,
How about
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
virtio_cread(..);
if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
....
}
>
>
>> + if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
>> + WRITE_ONCE(vb->report_free_page, false);
>> + } else if (cmd_id != vb->start_cmd_id) {
>> + /*
>> + * Host requests to start the reporting by sending a new cmd
>> + * id.
>> + */
>> + WRITE_ONCE(vb->report_free_page, true);
> I don't know why we bother with WRITE_ONCE here. The point of
> report_free_page being used lockless is that that it's not a big deal if
> it's wrong occasionally, right?
Actually the main reason is that "vb->report_free_page" is a value
shared by two threads:
Written by the config_change here, and read by the worker thread that
reports the free pages.
Alternatively, we could let the two sides access to the shared variable
with "volatile" pointers.
>
>
>
>> + vb->start_cmd_id = cmd_id;
>> + queue_work(vb->balloon_wq, &vb->report_free_page_work);
> It seems that if a command was already queued (with a different id),
> this will result in new command id being sent to host twice, which will
> likely confuse the host.
I think that case won't happen, because
- the host sends a cmd id to the guest via the config, while the guest
acks back the received cmd id via the virtqueue;
- the guest ack back a cmd id only when a new cmd id is received from
the host, that is the above check:
if (cmd_id != vb->start_cmd_id) { --> the driver only queues the
reporting work only when a new cmd id is received
/*
* Host requests to start the reporting by
sending a
* new cmd id.
*/
WRITE_ONCE(vb->report_free_page, true);
vb->start_cmd_id = cmd_id;
queue_work(vb->balloon_wq,
&vb->report_free_page_work);
}
So the same cmd id wouldn't queue the reporting work twice.
>
>
>
>> + }
>> +}
>> +
>> static void update_balloon_size(struct virtio_balloon *vb)
>> {
>> u32 actual = vb->num_pages;
>> @@ -417,40 +513,113 @@ static void update_balloon_size_func(struct work_struct *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;
>> + struct scatterlist sg;
>> + int i, nvqs, err = -ENOMEM;
>> +
>> + /* 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_FREE_PAGE_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;
> Why not just keep these 3 arrays on stack? they aren't large.
Sounds good. Here is the new implementation:
static int init_vqs(struct virtio_balloon *vb)
{
struct virtqueue *vqs[4];
vq_callback_t *callbacks[4];
const char *names[4];
struct scatterlist sg;
int ret;
/*
* Inflateq and deflateq are used unconditionally. stats_vq and
* free_page_vq uses names[2] and names[3], respectively. The
names[]
* will be NULL if the related feature is not enabled, which will
* cause no allocation for the corresponding virtqueue in find_vqs.
*/
callbacks[0] = balloon_ack;
names[0] = "inflate";
callbacks[1] = balloon_ack;
names[1] = "deflate";
names[2] = NULL;
names[3] = NULL;
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
names[2] = "stats";
callbacks[2] = stats_request;
}
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
names[3] = "free_page_vq";
callbacks[3] = NULL;
}
ret = vb->vdev->config->find_vqs(vb->vdev, 4, vqs, callbacks,
names,
NULL, NULL);
if (ret)
return ret;
vb->inflate_vq = vqs[0];
vb->deflate_vq = vqs[1];
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
vb->stats_vq = vqs[2];
/*
* Prime this virtqueue with one buffer so the
hypervisor can
* use it to signal us later (it can't be broken yet!).
*/
sg_init_one(&sg, vb->stats, sizeof(vb->stats));
ret = virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb,
GFP_KERNEL);
if (ret) {
dev_warn(&vb->vdev->dev, "%s: add stat_vq
failed\n",
__func__);
return ret;
}
virtqueue_kick(vb->stats_vq);
}
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ))
vb->free_page_vq = vqs[3];
return 0;
}
Btw, the QEMU side doesn't have an option to disable STATS_VQ currently,
we may need to add that later.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v22 3/3] virtio-balloon: don't report free pages when page poisoning is enabled
From: Michael S. Tsirkin @ 2018-01-18 22:37 UTC (permalink / raw)
To: Wei Wang
Cc: yang.zhang.wz, virtio-dev, riel, quan.xu0, kvm, nilal,
liliang.opensource, linux-kernel, mhocko, linux-mm, pbonzini,
akpm, virtualization
In-Reply-To: <1516165812-3995-4-git-send-email-wei.w.wang@intel.com>
On Wed, Jan 17, 2018 at 01:10:12PM +0800, Wei Wang wrote:
> The guest free pages should not be discarded by the live migration thread
> when page poisoning is enabled with PAGE_POISONING_NO_SANITY=n, because
> skipping the transfer of such poisoned free pages will trigger false
> positive when new pages are allocated and checked on the destination.
> This patch adds a config field, poison_val. Guest writes to the config
> field to tell the host about the poisoning value. The value will be 0 in
> the following cases:
> 1) PAGE_POISONING_NO_SANITY is enabled;
> 2) page poisoning is disabled; or
> 3) PAGE_POISONING_ZERO is enabled.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michal Hocko <mhocko@suse.com>
Pls squash with the previous patch. It's not nice to break a
config, then fix it up later.
> ---
> drivers/virtio/virtio_balloon.c | 8 ++++++++
> include/uapi/linux/virtio_balloon.h | 2 ++
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index b9561a5..5a42235 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -706,6 +706,7 @@ static struct file_system_type balloon_fs = {
> static int virtballoon_probe(struct virtio_device *vdev)
> {
> struct virtio_balloon *vb;
> + __u32 poison_val;
> int err;
>
> if (!vdev->config->get) {
> @@ -740,6 +741,13 @@ static int virtballoon_probe(struct virtio_device *vdev)
> WQ_FREEZABLE | WQ_CPU_INTENSIVE, 0);
> INIT_WORK(&vb->report_free_page_work, report_free_page_func);
> vb->stop_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID;
> + if (IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY) ||
> + !page_poisoning_enabled())
> + poison_val = 0;
> + else
> + poison_val = PAGE_POISON;
> + virtio_cwrite(vb->vdev, struct virtio_balloon_config,
> + poison_val, &poison_val);
> }
>
> vb->nb.notifier_call = virtballoon_oom_notify;
> diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
> index 55e2456..5861876 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -47,6 +47,8 @@ struct virtio_balloon_config {
> __u32 actual;
> /* Free page report command id, readonly by guest */
> __u32 free_page_report_cmd_id;
> + /* Stores PAGE_POISON if page poisoning with sanity check is in use */
> + __u32 poison_val;
> };
>
> #define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Michael S. Tsirkin @ 2018-01-18 22:32 UTC (permalink / raw)
To: Tetsuo Handa
Cc: yang.zhang.wz, virtio-dev, riel, quan.xu0, kvm, nilal,
liliang.opensource, linux-kernel, virtualization, linux-mm,
pbonzini, akpm, mhocko
In-Reply-To: <201801190611.HGI18722.FVtOMQLSHFFOOJ@I-love.SAKURA.ne.jp>
On Fri, Jan 19, 2018 at 06:11:31AM +0900, Tetsuo Handa wrote:
> Michael S. Tsirkin wrote:
> > On Thu, Jan 18, 2018 at 10:30:18PM +0900, Tetsuo Handa wrote:
> > > On 2018/01/18 1:44, Michael S. Tsirkin wrote:
> > > >> +static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t len)
> > > >> +{
> > > >> + struct scatterlist sg;
> > > >> + unsigned int unused;
> > > >> + int err;
> > > >> +
> > > >> + sg_init_table(&sg, 1);
> > > >> + sg_set_page(&sg, pfn_to_page(pfn), len, 0);
> > > >> +
> > > >> + /* Detach all the used buffers from the vq */
> > > >> + while (virtqueue_get_buf(vq, &unused))
> > > >> + ;
> > > >> +
> > > >> + /*
> > > >> + * Since this is an optimization feature, losing a couple of free
> > > >> + * pages to report isn't important.
> > > >> We simply resturn
> > > >
> > > > return
> > > >
> > > >> without adding
> > > >> + * the page if the vq is full. We are adding one entry each time,
> > > >> + * which essentially results in no memory allocation, so the
> > > >> + * GFP_KERNEL flag below can be ignored.
> > > >> + */
> > > >> + if (vq->num_free) {
> > > >> + err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL);
> > > >
> > > > Should we kick here? At least when ring is close to
> > > > being full. Kick at half way full?
> > > > Otherwise it's unlikely ring will
> > > > ever be cleaned until we finish the scan.
> > >
> > > Since this add_one_sg() is called between spin_lock_irqsave(&zone->lock, flags)
> > > and spin_unlock_irqrestore(&zone->lock, flags), it is not permitted to sleep.
> >
> > kick takes a while sometimes but it doesn't sleep.
>
> I don't know about virtio. But the purpose of kicking here is to wait for pending data
> to be flushed in order to increase vq->num_free, isn't it?
It isn't. It's to wake up device out of sleep to make it start
processing the pending data. If device isn't asleep, it's a nop.
> Then, doesn't waiting for
> pending data to be flushed involve sleeping? If yes, we can wait for completion of kick
> but we can't wait for completion of flush. Is pending data flushed without sleep?
>
> >
> > > And walk_free_mem_block() is not ready to handle resume.
> > >
> > > By the way, specifying GFP_KERNEL here is confusing even though it is never used.
> > > walk_free_mem_block() says:
> > >
> > > * The callback itself must not sleep or perform any operations which would
> > > * require any memory allocations directly (not even GFP_NOWAIT/GFP_ATOMIC)
> > > * or via any lock dependency.
> >
> > Yea, GFP_ATOMIC would do just as well. But I think any allocation
> > on this path would be problematic.
> >
> > How about a flag to make all allocations fail?
> >
> > E.g.
> >
> > #define GFP_FORBIDDEN (___GFP_DMA | ___GFP_HIGHMEM)
> >
> > Still this is not a blocker, we can worry about this later.
> >
> >
> > > >
> > > >> + /*
> > > >> + * This is expected to never fail, because there is always an
> > > >> + * entry available on the vq.
> > > >> + */
> > > >> + BUG_ON(err);
> > > >> + }
> > > >> +}
> >
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Michael S. Tsirkin @ 2018-01-18 19:09 UTC (permalink / raw)
To: Tetsuo Handa
Cc: yang.zhang.wz, virtio-dev, riel, quan.xu0, kvm, nilal,
liliang.opensource, linux-kernel, virtualization, linux-mm,
pbonzini, akpm, mhocko
In-Reply-To: <2bb0e3d9-1679-9ad3-b402-f0781f6cf094@I-love.SAKURA.ne.jp>
On Thu, Jan 18, 2018 at 10:30:18PM +0900, Tetsuo Handa wrote:
> On 2018/01/18 1:44, Michael S. Tsirkin wrote:
> >> +static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t len)
> >> +{
> >> + struct scatterlist sg;
> >> + unsigned int unused;
> >> + int err;
> >> +
> >> + sg_init_table(&sg, 1);
> >> + sg_set_page(&sg, pfn_to_page(pfn), len, 0);
> >> +
> >> + /* Detach all the used buffers from the vq */
> >> + while (virtqueue_get_buf(vq, &unused))
> >> + ;
> >> +
> >> + /*
> >> + * Since this is an optimization feature, losing a couple of free
> >> + * pages to report isn't important.
> >> We simply resturn
> >
> > return
> >
> >> without adding
> >> + * the page if the vq is full. We are adding one entry each time,
> >> + * which essentially results in no memory allocation, so the
> >> + * GFP_KERNEL flag below can be ignored.
> >> + */
> >> + if (vq->num_free) {
> >> + err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL);
> >
> > Should we kick here? At least when ring is close to
> > being full. Kick at half way full?
> > Otherwise it's unlikely ring will
> > ever be cleaned until we finish the scan.
>
> Since this add_one_sg() is called between spin_lock_irqsave(&zone->lock, flags)
> and spin_unlock_irqrestore(&zone->lock, flags), it is not permitted to sleep.
kick takes a while sometimes but it doesn't sleep.
> And walk_free_mem_block() is not ready to handle resume.
>
> By the way, specifying GFP_KERNEL here is confusing even though it is never used.
> walk_free_mem_block() says:
>
> * The callback itself must not sleep or perform any operations which would
> * require any memory allocations directly (not even GFP_NOWAIT/GFP_ATOMIC)
> * or via any lock dependency.
Yea, GFP_ATOMIC would do just as well. But I think any allocation
on this path would be problematic.
How about a flag to make all allocations fail?
E.g.
#define GFP_FORBIDDEN (___GFP_DMA | ___GFP_HIGHMEM)
Still this is not a blocker, we can worry about this later.
> >
> >> + /*
> >> + * This is expected to never fail, because there is always an
> >> + * entry available on the vq.
> >> + */
> >> + BUG_ON(err);
> >> + }
> >> +}
^ permalink raw reply
* CFP PhyCS 2018 - 5th Int.l Conf. on Physiological Computing Systems (Seville/Spain)
From: phycs @ 2018-01-18 17:18 UTC (permalink / raw)
To: virtualization
SUBMISSION DEADLINE
5th International Conference on Physiological Computing Systems
Submission Deadline: May 2, 2018
http://www.phycs.org/
September 19 - 21, 2018
Seville, Spain.
PhyCS is organized in 4 major tracks:
- Devices
- Methodologies and Methods
- Human Factors
- Applications
With the presence of internationally distinguished keynote speakers:
Eduardo Rocon, Consejo Superior de Investigaciones Científicas, Spain
Lucas Noldus, Noldus Information Technology bv, Netherlands
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
Should you have any question please don’t hesitate contacting me.
Kind regards,
PhyCS Secretariat
Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 184
Fax: +351 265 520 186
Web: http://www.phycs.org/
e-mail: phycs.secretariat@insticc.org
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* CFP WEBIST 2018 - 14th Int.l Conf. on Web Information Systems and Technologies (Seville/Spain)
From: webist @ 2018-01-18 17:17 UTC (permalink / raw)
To: virtualization
SUBMISSION DEADLINE
14th International Conference on Web Information Systems and Technologies
Submission Deadline: May 2, 2018
http://www.webist.org/
September 18 - 20, 2018
Seville, Spain.
WEBIST is organized in 5 major tracks:
- Internet Technology
- Mobile and NLP Information Systems
- Service Based Information Systems, Platforms and Eco-Systems
- Web Intelligence
- Web Interfaces
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
Should you have any question please don’t hesitate contacting me.
Kind regards,
WEBIST Secretariat
Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 100 033
Fax: +351 265 520 186
Web: http://www.webist.org/
e-mail: webist.secretariat@insticc.org
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [BUG] skcipher: Test 1 failed (invalid result) on encryption for cts(virtio_crypto_aes_cbc)
From: Corentin Labbe @ 2018-01-18 9:08 UTC (permalink / raw)
To: mst, jasowang, arei.gonglei, herbert, davem
Cc: linux-kernel, linux-crypto, virtualization
Hello
When modprobing tcrypt on a qemu virtual machine, I get the following trace in dmesg:
skcipher: Test 1 failed (invalid result) on encryption for cts(virtio_crypto_aes_cbc)
Regards
Corentin Labbe
^ permalink raw reply
* Re: [virtio-dev] [RFC PATCH net-next v2 1/2] virtio_net: Introduce VIRTIO_NET_F_BACKUP feature bit
From: Alexander Duyck @ 2018-01-17 21:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Duyck, Alexander H, virtio-dev, Jakub Kicinski,
Samudrala, Sridhar, virtualization, achiad shochat,
Achiad Shochat, Netdev, David Miller
In-Reply-To: <20180117213527-mutt-send-email-mst@kernel.org>
On Wed, Jan 17, 2018 at 11:57 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Jan 17, 2018 at 11:25:41AM -0800, Samudrala, Sridhar wrote:
>>
>>
>> On 1/17/2018 11:02 AM, Michael S. Tsirkin wrote:
>> > On Wed, Jan 17, 2018 at 10:15:52AM -0800, Alexander Duyck wrote:
>> > > On Thu, Jan 11, 2018 at 9:58 PM, Sridhar Samudrala
>> > > <sridhar.samudrala@intel.com> wrote:
>> > > > This feature bit can be used by hypervisor to indicate virtio_net device to
>> > > > act as a backup for another device with the same MAC address.
>> > > >
>> > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> > > > ---
>> > > > drivers/net/virtio_net.c | 2 +-
>> > > > include/uapi/linux/virtio_net.h | 3 +++
>> > > > 2 files changed, 4 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> > > > index 12dfc5fee58e..f149a160a8c5 100644
>> > > > --- a/drivers/net/virtio_net.c
>> > > > +++ b/drivers/net/virtio_net.c
>> > > > @@ -2829,7 +2829,7 @@ static struct virtio_device_id id_table[] = {
>> > > > VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>> > > > VIRTIO_NET_F_CTRL_MAC_ADDR, \
>> > > > VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
>> > > > - VIRTIO_NET_F_SPEED_DUPLEX
>> > > > + VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_BACKUP
>> > > >
>> > > > static unsigned int features[] = {
>> > > > VIRTNET_FEATURES,
>> > > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
>> > > > index 5de6ed37695b..c7c35fd1a5ed 100644
>> > > > --- a/include/uapi/linux/virtio_net.h
>> > > > +++ b/include/uapi/linux/virtio_net.h
>> > > > @@ -57,6 +57,9 @@
>> > > > * Steering */
>> > > > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>> > > >
>> > > > +#define VIRTIO_NET_F_BACKUP 62 /* Act as backup for another device
>> > > > + * with the same MAC.
>> > > > + */
>> > > > #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
>> > > >
>> > > > #ifndef VIRTIO_NET_NO_LEGACY
>> > > I'm not a huge fan of the name "backup" since that implies that the
>> > > Virtio interface is only used if the VF is not present, and there are
>> > > multiple instances such as dealing with east/west or
>> > > broadcast/multicast traffic where it may be desirable to use the
>> > > para-virtual interface rather then deal with PCI overhead/bottleneck
>> > > to send the packet.
>> > Right now hypervisors mostly expect that yes, only one at a time is
>> > used. E.g. if you try to do multicast sending packets on both VF and
>> > virtio then you will end up with two copies of each packet.
>>
>> I think we want to use only 1 interface to send out any packet. In case of
>> broadcast/multicasts it would be an optimization to send them via virtio and
>> this patch series adds that optimization.
>
> Right that's what I think we should rather avoid for now.
>
> It's *not* an optimization if there's a single VM on this host,
> or if a specific multicast group does not have any VMs on same
> host.
Agreed. In my mind this is something that is controlled by the
pass-thru interface once it is enslaved.
> I'd rather we just sent everything out on the PT if that's
> there. The reason we have virtio in the picture is just so
> we can migrate without downtime.
I wasn't saying we do that in all cases. That would be something that
would have to be decided by the pass-thru interface. Ideally the
virtio would provide just enough information to get itself into the
bond and I see this being the mechanism for it to do so. From there
the complexity mostly lies in the pass-thru interface to configure the
correct transmit modes if for example you have multiple pass-thru
interfaces or a more complex traffic setup due to things like
SwitchDev.
In my mind we go the bonding route and there are few use cases for all
of this. First is the backup case that is being addressed here. That
becomes your basic "copy netvsc" approach for this which would be
default. It is how we would handle basic pass-thru back-up paths. If
the host decides to send multicast/broadcast traffic from the host up
through it that is a host side decision. I am okay with our default
transmit behavior from the guest being to send everything through the
pass-thru interface. All the virtio would be doing is advertising that
it is a side channel for some sort of bond with another interface. By
calling it a side channel it implies that it isn't the direct channel
for the interface, but it is an alternative communications channel for
things like backup, and other future uses.
There end up being a few different "phase 2" concepts I have floating
around which I want to avoid limiting. Solving the east/west problem
is an example. In my mind that just becomes a bonding Tx mode that is
controlled via the pass-thru interface. The VF could black-list the
local addresses so that they instead fall into the virtio interface.
In addition I seem to recall watching a presentation from Mellanox
where they were talking about a VF per NUMA node in a system which
would imply multiple VFs with the same MAC address. I'm not sure how
the current patch handles that. Obviously that would require a more
complex load balancing setup. The bonding solution ends up being a way
to resolve that so that they could just have it take care of picking
the right Tx queue based on the NUMA affinity and fall back to the
virtio/netvsc when those fail.
>> In the receive path, the broadcasts should only go the PF and reach the VM
>> via vitio so that the VM doesn't see duplicate broadcasts.
>>
>>
>> >
>> > To me the east/west scenario looks like you want something
>> > more similar to a bridge on top of the virtio/PT pair.
>> >
>> > So I suspect that use-case will need a separate configuration bit,
>> > and possibly that's when you will want something more powerful
>> > such as a full bridge.
>>
>> east-west optimization of unicasts would be a harder problem to solve as
>> somehow the hypervisor needs to indicate the VM about the local dest. macs
>
> Using a bridge with a dedicated device for east/west would let
> bridge use standard learning techniques for that perhaps?
I'm not sure about having the guest have to learn. In my mind the VF
should know who is local and who isn't. In the case of SwitchDev it
should be possible for the port representors and the switch to provide
data on which interfaces are bonded on the host side and which aren't.
With that data it would be pretty easy to just put together a list of
addresses that would prefer to go the para-virtual route instead of
being transmitted through physical hardware.
In addition a bridge implies much more overhead since normally a
bridge can receive a packet in on one interface and transmit it on
another. We don't really need that. This is more of a VEPA type setup
and doesn't need to be anything all that complex. You could probably
even handle the Tx queue selection via a simple eBPF program and map
since the input for whatever is used to select Tx should be pretty
simple, destination MAC, source NUMA node, etc, and the data-set
shouldn't be too large.
>> > > What if instead of BACKUP we used the name SIDE_CHANNEL? Basically it
>> > > is a bit of double entendre as we are using the physical MAC address
>> > > to provide configuration information, and then in addition this
>> > > interface acts as a secondary channel for passing frames to and from
>> > > the guest rather than just using the VF.
>> > >
>> > > Just a thought.
>> > >
>> > > Thanks.
>> > >
>> > > - Alex
>> > I just feel that's a very generic name, not conveying enough information
>> > about how they hypervisor expects the pair of devices to be used.
I would argue that BACKUP is too specific. I can see many other uses
other than just being a backup interface and I don't want us to box
ourselves in. I agree that it makes sense for active/backup to be the
base mode, but I really don't think it conveys all of the
possibilities since I see this as possibly being able to solve
multiple issues as this evolves. I'm also open to other suggestions.
We could go with PASSTHRU_SIDE_CHANNEL for instance, I just didn't
suggest that since it seemed kind of wordy.
^ permalink raw reply
* Re: [virtio-dev] [RFC PATCH net-next v2 1/2] virtio_net: Introduce VIRTIO_NET_F_BACKUP feature bit
From: Michael S. Tsirkin @ 2018-01-17 19:57 UTC (permalink / raw)
To: Samudrala, Sridhar
Cc: Duyck, Alexander H, virtio-dev, Jakub Kicinski, Netdev,
Alexander Duyck, virtualization, David Miller
In-Reply-To: <058068e5-febd-92c8-e5a9-faf262b82335@intel.com>
On Wed, Jan 17, 2018 at 11:25:41AM -0800, Samudrala, Sridhar wrote:
>
>
> On 1/17/2018 11:02 AM, Michael S. Tsirkin wrote:
> > On Wed, Jan 17, 2018 at 10:15:52AM -0800, Alexander Duyck wrote:
> > > On Thu, Jan 11, 2018 at 9:58 PM, Sridhar Samudrala
> > > <sridhar.samudrala@intel.com> wrote:
> > > > This feature bit can be used by hypervisor to indicate virtio_net device to
> > > > act as a backup for another device with the same MAC address.
> > > >
> > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> > > > ---
> > > > drivers/net/virtio_net.c | 2 +-
> > > > include/uapi/linux/virtio_net.h | 3 +++
> > > > 2 files changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 12dfc5fee58e..f149a160a8c5 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -2829,7 +2829,7 @@ static struct virtio_device_id id_table[] = {
> > > > VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> > > > VIRTIO_NET_F_CTRL_MAC_ADDR, \
> > > > VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> > > > - VIRTIO_NET_F_SPEED_DUPLEX
> > > > + VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_BACKUP
> > > >
> > > > static unsigned int features[] = {
> > > > VIRTNET_FEATURES,
> > > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > > > index 5de6ed37695b..c7c35fd1a5ed 100644
> > > > --- a/include/uapi/linux/virtio_net.h
> > > > +++ b/include/uapi/linux/virtio_net.h
> > > > @@ -57,6 +57,9 @@
> > > > * Steering */
> > > > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> > > >
> > > > +#define VIRTIO_NET_F_BACKUP 62 /* Act as backup for another device
> > > > + * with the same MAC.
> > > > + */
> > > > #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
> > > >
> > > > #ifndef VIRTIO_NET_NO_LEGACY
> > > I'm not a huge fan of the name "backup" since that implies that the
> > > Virtio interface is only used if the VF is not present, and there are
> > > multiple instances such as dealing with east/west or
> > > broadcast/multicast traffic where it may be desirable to use the
> > > para-virtual interface rather then deal with PCI overhead/bottleneck
> > > to send the packet.
> > Right now hypervisors mostly expect that yes, only one at a time is
> > used. E.g. if you try to do multicast sending packets on both VF and
> > virtio then you will end up with two copies of each packet.
>
> I think we want to use only 1 interface to send out any packet. In case of
> broadcast/multicasts it would be an optimization to send them via virtio and
> this patch series adds that optimization.
Right that's what I think we should rather avoid for now.
It's *not* an optimization if there's a single VM on this host,
or if a specific multicast group does not have any VMs on same
host.
I'd rather we just sent everything out on the PT if that's
there. The reason we have virtio in the picture is just so
we can migrate without downtime.
> In the receive path, the broadcasts should only go the PF and reach the VM
> via vitio so that the VM doesn't see duplicate broadcasts.
>
>
> >
> > To me the east/west scenario looks like you want something
> > more similar to a bridge on top of the virtio/PT pair.
> >
> > So I suspect that use-case will need a separate configuration bit,
> > and possibly that's when you will want something more powerful
> > such as a full bridge.
>
> east-west optimization of unicasts would be a harder problem to solve as
> somehow the hypervisor needs to indicate the VM about the local dest. macs
Using a bridge with a dedicated device for east/west would let
bridge use standard learning techniques for that perhaps?
> >
> >
> > > What if instead of BACKUP we used the name SIDE_CHANNEL? Basically it
> > > is a bit of double entendre as we are using the physical MAC address
> > > to provide configuration information, and then in addition this
> > > interface acts as a secondary channel for passing frames to and from
> > > the guest rather than just using the VF.
> > >
> > > Just a thought.
> > >
> > > Thanks.
> > >
> > > - Alex
> > I just feel that's a very generic name, not conveying enough information
> > about how they hypervisor expects the pair of devices to be used.
> >
^ permalink raw reply
* Re: [virtio-dev] [RFC PATCH net-next v2 1/2] virtio_net: Introduce VIRTIO_NET_F_BACKUP feature bit
From: Samudrala, Sridhar @ 2018-01-17 19:25 UTC (permalink / raw)
To: Michael S. Tsirkin, Alexander Duyck
Cc: Duyck, Alexander H, virtio-dev, Jakub Kicinski, Netdev,
virtualization, David Miller
In-Reply-To: <20180117203757-mutt-send-email-mst@kernel.org>
On 1/17/2018 11:02 AM, Michael S. Tsirkin wrote:
> On Wed, Jan 17, 2018 at 10:15:52AM -0800, Alexander Duyck wrote:
>> On Thu, Jan 11, 2018 at 9:58 PM, Sridhar Samudrala
>> <sridhar.samudrala@intel.com> wrote:
>>> This feature bit can be used by hypervisor to indicate virtio_net device to
>>> act as a backup for another device with the same MAC address.
>>>
>>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>> ---
>>> drivers/net/virtio_net.c | 2 +-
>>> include/uapi/linux/virtio_net.h | 3 +++
>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index 12dfc5fee58e..f149a160a8c5 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -2829,7 +2829,7 @@ static struct virtio_device_id id_table[] = {
>>> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>>> VIRTIO_NET_F_CTRL_MAC_ADDR, \
>>> VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
>>> - VIRTIO_NET_F_SPEED_DUPLEX
>>> + VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_BACKUP
>>>
>>> static unsigned int features[] = {
>>> VIRTNET_FEATURES,
>>> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
>>> index 5de6ed37695b..c7c35fd1a5ed 100644
>>> --- a/include/uapi/linux/virtio_net.h
>>> +++ b/include/uapi/linux/virtio_net.h
>>> @@ -57,6 +57,9 @@
>>> * Steering */
>>> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>>>
>>> +#define VIRTIO_NET_F_BACKUP 62 /* Act as backup for another device
>>> + * with the same MAC.
>>> + */
>>> #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
>>>
>>> #ifndef VIRTIO_NET_NO_LEGACY
>> I'm not a huge fan of the name "backup" since that implies that the
>> Virtio interface is only used if the VF is not present, and there are
>> multiple instances such as dealing with east/west or
>> broadcast/multicast traffic where it may be desirable to use the
>> para-virtual interface rather then deal with PCI overhead/bottleneck
>> to send the packet.
> Right now hypervisors mostly expect that yes, only one at a time is
> used. E.g. if you try to do multicast sending packets on both VF and
> virtio then you will end up with two copies of each packet.
I think we want to use only 1 interface to send out any packet. In case of
broadcast/multicasts it would be an optimization to send them via virtio and
this patch series adds that optimization.
In the receive path, the broadcasts should only go the PF and reach the VM
via vitio so that the VM doesn't see duplicate broadcasts.
>
> To me the east/west scenario looks like you want something
> more similar to a bridge on top of the virtio/PT pair.
>
> So I suspect that use-case will need a separate configuration bit,
> and possibly that's when you will want something more powerful
> such as a full bridge.
east-west optimization of unicasts would be a harder problem to solve as
somehow the hypervisor needs to indicate the VM about the local dest. macs
>
>
>> What if instead of BACKUP we used the name SIDE_CHANNEL? Basically it
>> is a bit of double entendre as we are using the physical MAC address
>> to provide configuration information, and then in addition this
>> interface acts as a secondary channel for passing frames to and from
>> the guest rather than just using the VF.
>>
>> Just a thought.
>>
>> Thanks.
>>
>> - Alex
> I just feel that's a very generic name, not conveying enough information
> about how they hypervisor expects the pair of devices to be used.
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [virtio-dev] [RFC PATCH net-next v2 1/2] virtio_net: Introduce VIRTIO_NET_F_BACKUP feature bit
From: Michael S. Tsirkin @ 2018-01-17 19:02 UTC (permalink / raw)
To: Alexander Duyck
Cc: Duyck, Alexander H, virtio-dev, Jakub Kicinski, Sridhar Samudrala,
virtualization, Netdev, David Miller
In-Reply-To: <CAKgT0Uc8bRoAsXYSr7k27gf5+vh7rF2Dd_kWNB1d38tpZAeRGg@mail.gmail.com>
On Wed, Jan 17, 2018 at 10:15:52AM -0800, Alexander Duyck wrote:
> On Thu, Jan 11, 2018 at 9:58 PM, Sridhar Samudrala
> <sridhar.samudrala@intel.com> wrote:
> > This feature bit can be used by hypervisor to indicate virtio_net device to
> > act as a backup for another device with the same MAC address.
> >
> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> > ---
> > drivers/net/virtio_net.c | 2 +-
> > include/uapi/linux/virtio_net.h | 3 +++
> > 2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 12dfc5fee58e..f149a160a8c5 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2829,7 +2829,7 @@ static struct virtio_device_id id_table[] = {
> > VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> > VIRTIO_NET_F_CTRL_MAC_ADDR, \
> > VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> > - VIRTIO_NET_F_SPEED_DUPLEX
> > + VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_BACKUP
> >
> > static unsigned int features[] = {
> > VIRTNET_FEATURES,
> > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > index 5de6ed37695b..c7c35fd1a5ed 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -57,6 +57,9 @@
> > * Steering */
> > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> >
> > +#define VIRTIO_NET_F_BACKUP 62 /* Act as backup for another device
> > + * with the same MAC.
> > + */
> > #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
> >
> > #ifndef VIRTIO_NET_NO_LEGACY
>
> I'm not a huge fan of the name "backup" since that implies that the
> Virtio interface is only used if the VF is not present, and there are
> multiple instances such as dealing with east/west or
> broadcast/multicast traffic where it may be desirable to use the
> para-virtual interface rather then deal with PCI overhead/bottleneck
> to send the packet.
Right now hypervisors mostly expect that yes, only one at a time is
used. E.g. if you try to do multicast sending packets on both VF and
virtio then you will end up with two copies of each packet.
To me the east/west scenario looks like you want something
more similar to a bridge on top of the virtio/PT pair.
So I suspect that use-case will need a separate configuration bit,
and possibly that's when you will want something more powerful
such as a full bridge.
> What if instead of BACKUP we used the name SIDE_CHANNEL? Basically it
> is a bit of double entendre as we are using the physical MAC address
> to provide configuration information, and then in addition this
> interface acts as a secondary channel for passing frames to and from
> the guest rather than just using the VF.
>
> Just a thought.
>
> Thanks.
>
> - Alex
I just feel that's a very generic name, not conveying enough information
about how they hypervisor expects the pair of devices to be used.
--
MST
^ permalink raw reply
* Re: [virtio-dev] [RFC PATCH net-next v2 1/2] virtio_net: Introduce VIRTIO_NET_F_BACKUP feature bit
From: Alexander Duyck @ 2018-01-17 18:15 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: Duyck, Alexander H, virtio-dev, Michael S. Tsirkin,
Jakub Kicinski, Netdev, virtualization, David Miller
In-Reply-To: <1515736720-39368-2-git-send-email-sridhar.samudrala@intel.com>
On Thu, Jan 11, 2018 at 9:58 PM, Sridhar Samudrala
<sridhar.samudrala@intel.com> wrote:
> This feature bit can be used by hypervisor to indicate virtio_net device to
> act as a backup for another device with the same MAC address.
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
> drivers/net/virtio_net.c | 2 +-
> include/uapi/linux/virtio_net.h | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 12dfc5fee58e..f149a160a8c5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2829,7 +2829,7 @@ static struct virtio_device_id id_table[] = {
> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> VIRTIO_NET_F_CTRL_MAC_ADDR, \
> VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> - VIRTIO_NET_F_SPEED_DUPLEX
> + VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_BACKUP
>
> static unsigned int features[] = {
> VIRTNET_FEATURES,
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index 5de6ed37695b..c7c35fd1a5ed 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -57,6 +57,9 @@
> * Steering */
> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>
> +#define VIRTIO_NET_F_BACKUP 62 /* Act as backup for another device
> + * with the same MAC.
> + */
> #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
>
> #ifndef VIRTIO_NET_NO_LEGACY
I'm not a huge fan of the name "backup" since that implies that the
Virtio interface is only used if the VF is not present, and there are
multiple instances such as dealing with east/west or
broadcast/multicast traffic where it may be desirable to use the
para-virtual interface rather then deal with PCI overhead/bottleneck
to send the packet.
What if instead of BACKUP we used the name SIDE_CHANNEL? Basically it
is a bit of double entendre as we are using the physical MAC address
to provide configuration information, and then in addition this
interface acts as a secondary channel for passing frames to and from
the guest rather than just using the VF.
Just a thought.
Thanks.
- Alex
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Michael S. Tsirkin @ 2018-01-17 16:44 UTC (permalink / raw)
To: Wei Wang
Cc: yang.zhang.wz, virtio-dev, riel, quan.xu0, kvm, nilal,
liliang.opensource, linux-kernel, mhocko, linux-mm, pbonzini,
akpm, virtualization
In-Reply-To: <1516165812-3995-3-git-send-email-wei.w.wang@intel.com>
On Wed, Jan 17, 2018 at 01:10:11PM +0800, Wei Wang wrote:
> Negotiation of the VIRTIO_BALLOON_F_FREE_PAGE_VQ feature indicates the
> support of reporting hints of guest free pages to host via virtio-balloon.
>
> Host requests the guest to report free pages by sending a new cmd
> id to the guest via the free_page_report_cmd_id configuration register.
>
> When the guest starts to report, the first element added to the free page
> vq is the cmd id given by host. When the guest finishes the reporting
> of all the free pages, VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID is added
> to the vq to tell host that the reporting is done. Host may also requests
> the guest to stop the reporting in advance by sending the stop cmd id to
> the guest via the configuration register.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> ---
> drivers/virtio/virtio_balloon.c | 242 +++++++++++++++++++++++++++++++-----
> include/uapi/linux/virtio_balloon.h | 4 +
> 2 files changed, 214 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index a1fb52c..b9561a5 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -53,7 +53,12 @@ static struct vfsmount *balloon_mnt;
>
> struct virtio_balloon {
> struct virtio_device *vdev;
> - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq;
> +
> + /* Balloon's own wq for cpu-intensive work items */
> + struct workqueue_struct *balloon_wq;
> + /* The free page reporting work item submitted to the balloon wq */
> + struct work_struct report_free_page_work;
>
> /* The balloon servicing is delegated to a freezable workqueue. */
> struct work_struct update_balloon_stats_work;
> @@ -63,6 +68,13 @@ struct virtio_balloon {
> spinlock_t stop_update_lock;
> bool stop_update;
>
> + /* Start to report free pages */
> + bool report_free_page;
> + /* Stores the cmd id given by host to start the free page reporting */
> + uint32_t start_cmd_id;
> + /* Stores STOP_ID as a sign to tell host that the reporting is done */
> + uint32_t stop_cmd_id;
> +
> /* Waiting for host to ack the pages we released. */
> wait_queue_head_t acked;
>
> @@ -281,6 +293,71 @@ static unsigned int update_balloon_stats(struct virtio_balloon *vb)
> return idx;
> }
>
> +static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t len)
> +{
> + struct scatterlist sg;
> + unsigned int unused;
> + int err;
> +
> + sg_init_table(&sg, 1);
> + sg_set_page(&sg, pfn_to_page(pfn), len, 0);
> +
> + /* Detach all the used buffers from the vq */
> + while (virtqueue_get_buf(vq, &unused))
> + ;
> +
> + /*
> + * Since this is an optimization feature, losing a couple of free
> + * pages to report isn't important.
> We simply resturn
return
> without adding
> + * the page if the vq is full. We are adding one entry each time,
> + * which essentially results in no memory allocation, so the
> + * GFP_KERNEL flag below can be ignored.
> + */
> + if (vq->num_free) {
> + err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL);
Should we kick here? At least when ring is close to
being full. Kick at half way full?
Otherwise it's unlikely ring will
ever be cleaned until we finish the scan.
> + /*
> + * This is expected to never fail, because there is always an
> + * entry available on the vq.
> + */
> + BUG_ON(err);
> + }
> +}
> +
> +static void batch_free_page_sg(struct virtqueue *vq,
> + unsigned long pfn,
> + uint32_t len)
Not sure what does batch refer to here.
I'd just open-code this.
> +{
> + add_one_sg(vq, pfn, len);
> +
> + /* Batch till the vq is full */
> + if (!vq->num_free)
> + virtqueue_kick(vq);
> +}
> +
> +static void send_cmd_id(struct virtqueue *vq, void *addr)
Why void *? Should be a specific type.
then you can use sizeof *addr as size.
> +{
> + struct scatterlist sg;
> + unsigned int unused;
> + int err;
> +
> + sg_init_one(&sg, addr, sizeof(uint32_t));
This passes a guest-endian value to host. This is a problem:
should always pass LE values.
> +
> + /*
> + * This handles the cornercase that the vq happens to be full when
> + * adding a cmd id. Rarely happen in practice.
> + */
> + while (!vq->num_free)
> + virtqueue_get_buf(vq, &unused);
I dislike this busy-waiting. It's a hint after all -
why not just retry later - hopefully after getting an
interrupt?
Alternatively, stop adding more entries when we have a single
ring entry left, making sure we have space for the command.
> +
> + err = virtqueue_add_outbuf(vq, &sg, 1, vq, GFP_KERNEL);
> + /*
> + * This is expected to never fail, because there is always an
> + * entry available on the vq.
> + */
> + BUG_ON(err);
> + virtqueue_kick(vq);
> +}
> +
> /*
> * While most virtqueues communicate guest-initiated requests to the hypervisor,
> * the stats queue operates in reverse. The driver initializes the virtqueue
> @@ -316,17 +393,6 @@ static void stats_handle_request(struct virtio_balloon *vb)
> virtqueue_kick(vq);
> }
>
> -static void virtballoon_changed(struct virtio_device *vdev)
> -{
> - struct virtio_balloon *vb = vdev->priv;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&vb->stop_update_lock, flags);
> - if (!vb->stop_update)
> - queue_work(system_freezable_wq, &vb->update_balloon_size_work);
> - spin_unlock_irqrestore(&vb->stop_update_lock, flags);
> -}
> -
> static inline s64 towards_target(struct virtio_balloon *vb)
> {
> s64 target;
> @@ -343,6 +409,36 @@ static inline s64 towards_target(struct virtio_balloon *vb)
> return target - vb->num_pages;
> }
>
> +static void virtballoon_changed(struct virtio_device *vdev)
> +{
> + struct virtio_balloon *vb = vdev->priv;
> + unsigned long flags;
> + __u32 cmd_id;
> + s64 diff = towards_target(vb);
> +
> + if (diff) {
> + spin_lock_irqsave(&vb->stop_update_lock, flags);
> + if (!vb->stop_update)
Why do you ignore stop_update for freeze?
This means new wq entries can be added during remove
causing use after free issues.
> + queue_work(system_freezable_wq,
> + &vb->update_balloon_size_work);
> + spin_unlock_irqrestore(&vb->stop_update_lock, flags);
> + }
> +
> + virtio_cread(vb->vdev, struct virtio_balloon_config,
> + free_page_report_cmd_id, &cmd_id);
You want virtio_cread_feature, don't access the new field
if the feature has not been negotiated.
> + if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
> + WRITE_ONCE(vb->report_free_page, false);
> + } else if (cmd_id != vb->start_cmd_id) {
> + /*
> + * Host requests to start the reporting by sending a new cmd
> + * id.
> + */
> + WRITE_ONCE(vb->report_free_page, true);
I don't know why we bother with WRITE_ONCE here. The point of
report_free_page being used lockless is that that it's not a big deal if
it's wrong occasionally, right?
> + vb->start_cmd_id = cmd_id;
> + queue_work(vb->balloon_wq, &vb->report_free_page_work);
It seems that if a command was already queued (with a different id),
this will result in new command id being sent to host twice, which will
likely confuse the host.
> + }
> +}
> +
> static void update_balloon_size(struct virtio_balloon *vb)
> {
> u32 actual = vb->num_pages;
> @@ -417,40 +513,113 @@ static void update_balloon_size_func(struct work_struct *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;
> + struct scatterlist sg;
> + int i, nvqs, err = -ENOMEM;
> +
> + /* 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_FREE_PAGE_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;
Why not just keep these 3 arrays on stack? they aren't large.
> +
> + 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 = virtio_find_vqs(vb->vdev, nvqs, vqs, callbacks, names, NULL);
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
> + callbacks[i] = NULL;
> + names[i] = "free_page_vq";
> + }
> +
> + err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names,
> + NULL, 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();
> + < 0) {
> + dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n",
> + __func__);
> + goto err_find;
> + }
> virtqueue_kick(vb->stats_vq);
> }
> +
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ))
> + vb->free_page_vq = vqs[i];
> +
> + kfree(names);
> + kfree(callbacks);
> + kfree(vqs);
> return 0;
> +
> +err_find:
> + kfree(names);
> +err_names:
> + kfree(callbacks);
> +err_callback:
> + kfree(vqs);
> +err_vq:
> + return err;
> +}
> +
> +static bool virtio_balloon_send_free_pages(void *opaque, unsigned long pfn,
> + unsigned long nr_pages)
> +{
> + struct virtio_balloon *vb = (struct virtio_balloon *)opaque;
> + uint32_t len = nr_pages << PAGE_SHIFT;
> +
> + if (!READ_ONCE(vb->report_free_page))
> + return false;
> +
> + batch_free_page_sg(vb->free_page_vq, pfn, len);
> +
> + return true;
> +}
> +
> +static void report_free_page_func(struct work_struct *work)
> +{
> + struct virtio_balloon *vb;
> +
> + vb = container_of(work, struct virtio_balloon, report_free_page_work);
> + /* Start by sending the obtained cmd id to the host with an outbuf */
> + send_cmd_id(vb->free_page_vq, &vb->start_cmd_id);
> + walk_free_mem_block(vb, 0, &virtio_balloon_send_free_pages);
> + /* End by sending the stop id to the host with an outbuf */
> + send_cmd_id(vb->free_page_vq, &vb->stop_cmd_id);
> }
>
> #ifdef CONFIG_BALLOON_COMPACTION
> @@ -566,6 +735,13 @@ static int virtballoon_probe(struct virtio_device *vdev)
> if (err)
> goto out_free_vb;
>
> + if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
> + vb->balloon_wq = alloc_workqueue("balloon-wq",
> + WQ_FREEZABLE | WQ_CPU_INTENSIVE, 0);
No destroy_workqueue in sight, will likely leak a wq in remove.
> + INIT_WORK(&vb->report_free_page_work, report_free_page_func);
> + vb->stop_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID;
> + }
> +
> vb->nb.notifier_call = virtballoon_oom_notify;
> vb->nb.priority = VIRTBALLOON_OOM_NOTIFY_PRIORITY;
> err = register_oom_notifier(&vb->nb);
> @@ -630,6 +806,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->report_free_page_work);
>
> remove_common(vb);
> #ifdef CONFIG_BALLOON_COMPACTION
> @@ -682,6 +859,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_FREE_PAGE_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 343d7dd..55e2456 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -34,15 +34,19 @@
> #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_FREE_PAGE_VQ 3 /* VQ to report free pages */
>
> /* Size of a PFN in the balloon interface. */
> #define VIRTIO_BALLOON_PFN_SHIFT 12
>
> +#define VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID 0
> struct virtio_balloon_config {
> /* Number of pages host wants Guest to give up. */
> __u32 num_pages;
> /* Number of pages we've actually got in balloon. */
> __u32 actual;
> + /* Free page report command id, readonly by guest */
> + __u32 free_page_report_cmd_id;
> };
>
> #define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Wei Wang @ 2018-01-17 10:47 UTC (permalink / raw)
To: Pankaj Gupta
Cc: yang zhang wz, virtio-dev, riel, quan xu0, kvm, mst, nilal,
liliang opensource, linux-kernel, virtualization, linux-mm,
pbonzini, akpm, mhocko
In-Reply-To: <1239524301.1023371.1516181271621.JavaMail.zimbra@redhat.com>
On 01/17/2018 05:27 PM, Pankaj Gupta wrote:
>> On 01/17/2018 04:21 PM, Pankaj Gupta wrote:
>>
> o.k you have initialize "err = -ENOMEM;"
>
> Remove these four lines.
>
> - kfree(names);
> - kfree(callbacks);
> - kfree(vqs);
> - return 0;
>
> + err = 0; // if executed without any error
>
OK, thanks. "error = 0" is not needed actually.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Pankaj Gupta @ 2018-01-17 9:27 UTC (permalink / raw)
To: Wei Wang
Cc: yang zhang wz, virtio-dev, riel, quan xu0, kvm, mst, nilal,
liliang opensource, linux-kernel, virtualization, linux-mm,
pbonzini, akpm, mhocko
In-Reply-To: <5A5F109B.7090200@intel.com>
> On 01/17/2018 04:21 PM, Pankaj Gupta wrote:
> >> Negotiation of the VIRTIO_BALLOON_F_FREE_PAGE_VQ feature indicates the
> >> support of reporting hints of guest free pages to host via virtio-balloon.
> >>
> >> Host requests the guest to report free pages by sending a new cmd
> >> id to the guest via the free_page_report_cmd_id configuration register.
> >>
> >> When the guest starts to report, the first element added to the free page
> >> vq is the cmd id given by host. When the guest finishes the reporting
> >> of all the free pages, VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID is added
> >> to the vq to tell host that the reporting is done. Host may also requests
> >> the guest to stop the reporting in advance by sending the stop cmd id to
> >> the guest via the configuration register.
> >>
> >> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> >> Signed-off-by: Liang Li <liang.z.li@intel.com>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Cc: Michal Hocko <mhocko@kernel.org>
> >> ---
> >> drivers/virtio/virtio_balloon.c | 242
> >> +++++++++++++++++++++++++++++++-----
> >> include/uapi/linux/virtio_balloon.h | 4 +
> >> 2 files changed, 214 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/drivers/virtio/virtio_balloon.c
> >> b/drivers/virtio/virtio_balloon.c
> >> index a1fb52c..b9561a5 100644
> >> --- a/drivers/virtio/virtio_balloon.c
> >> +++ b/drivers/virtio/virtio_balloon.c
> >> @@ -53,7 +53,12 @@ static struct vfsmount *balloon_mnt;
> >>
> >> struct virtio_balloon {
> >> struct virtio_device *vdev;
> >> - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> >> + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq;
> >> +
> >> + /* Balloon's own wq for cpu-intensive work items */
> >> + struct workqueue_struct *balloon_wq;
> >> + /* The free page reporting work item submitted to the balloon wq */
> >> + struct work_struct report_free_page_work;
> >>
> >> /* The balloon servicing is delegated to a freezable workqueue. */
> >> struct work_struct update_balloon_stats_work;
> >> @@ -63,6 +68,13 @@ struct virtio_balloon {
> >> spinlock_t stop_update_lock;
> >> bool stop_update;
> >>
> >> + /* Start to report free pages */
> >> + bool report_free_page;
> >> + /* Stores the cmd id given by host to start the free page reporting */
> >> + uint32_t start_cmd_id;
> >> + /* Stores STOP_ID as a sign to tell host that the reporting is done */
> >> + uint32_t stop_cmd_id;
> >> +
> >> /* Waiting for host to ack the pages we released. */
> >> wait_queue_head_t acked;
> >>
> >> @@ -281,6 +293,71 @@ static unsigned int update_balloon_stats(struct
> >> virtio_balloon *vb)
> >> return idx;
> >> }
> >>
> >> +static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t
> >> len)
> >> +{
> >> + struct scatterlist sg;
> >> + unsigned int unused;
> >> + int err;
> >> +
> >> + sg_init_table(&sg, 1);
> >> + sg_set_page(&sg, pfn_to_page(pfn), len, 0);
> >> +
> >> + /* Detach all the used buffers from the vq */
> >> + while (virtqueue_get_buf(vq, &unused))
> >> + ;
> >> +
> >> + /*
> >> + * Since this is an optimization feature, losing a couple of free
> >> + * pages to report isn't important. We simply resturn without adding
> >> + * the page if the vq is full. We are adding one entry each time,
> >> + * which essentially results in no memory allocation, so the
> >> + * GFP_KERNEL flag below can be ignored.
> >> + */
> >> + if (vq->num_free) {
> >> + err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL);
> >> + /*
> >> + * This is expected to never fail, because there is always an
> >> + * entry available on the vq.
> >> + */
> >> + BUG_ON(err);
> >> + }
> >> +}
> >> +
> >> +static void batch_free_page_sg(struct virtqueue *vq,
> >> + unsigned long pfn,
> >> + uint32_t len)
> >> +{
> >> + add_one_sg(vq, pfn, len);
> >> +
> >> + /* Batch till the vq is full */
> >> + if (!vq->num_free)
> >> + virtqueue_kick(vq);
> >> +}
> >> +
> >> +static void send_cmd_id(struct virtqueue *vq, void *addr)
> >> +{
> >> + struct scatterlist sg;
> >> + unsigned int unused;
> >> + int err;
> >> +
> >> + sg_init_one(&sg, addr, sizeof(uint32_t));
> >> +
> >> + /*
> >> + * This handles the cornercase that the vq happens to be full when
> >> + * adding a cmd id. Rarely happen in practice.
> >> + */
> >> + while (!vq->num_free)
> >> + virtqueue_get_buf(vq, &unused);
> >> +
> >> + err = virtqueue_add_outbuf(vq, &sg, 1, vq, GFP_KERNEL);
> >> + /*
> >> + * This is expected to never fail, because there is always an
> >> + * entry available on the vq.
> >> + */
> >> + BUG_ON(err);
> >> + virtqueue_kick(vq);
> >> +}
> >> +
> >> /*
> >> * While most virtqueues communicate guest-initiated requests to the
> >> hypervisor,
> >> * the stats queue operates in reverse. The driver initializes the
> >> virtqueue
> >> @@ -316,17 +393,6 @@ static void stats_handle_request(struct
> >> virtio_balloon
> >> *vb)
> >> virtqueue_kick(vq);
> >> }
> >>
> >> -static void virtballoon_changed(struct virtio_device *vdev)
> >> -{
> >> - struct virtio_balloon *vb = vdev->priv;
> >> - unsigned long flags;
> >> -
> >> - spin_lock_irqsave(&vb->stop_update_lock, flags);
> >> - if (!vb->stop_update)
> >> - queue_work(system_freezable_wq, &vb->update_balloon_size_work);
> >> - spin_unlock_irqrestore(&vb->stop_update_lock, flags);
> >> -}
> >> -
> >> static inline s64 towards_target(struct virtio_balloon *vb)
> >> {
> >> s64 target;
> >> @@ -343,6 +409,36 @@ static inline s64 towards_target(struct
> >> virtio_balloon
> >> *vb)
> >> return target - vb->num_pages;
> >> }
> >>
> >> +static void virtballoon_changed(struct virtio_device *vdev)
> >> +{
> >> + struct virtio_balloon *vb = vdev->priv;
> >> + unsigned long flags;
> >> + __u32 cmd_id;
> >> + s64 diff = towards_target(vb);
> >> +
> >> + if (diff) {
> >> + spin_lock_irqsave(&vb->stop_update_lock, flags);
> >> + if (!vb->stop_update)
> >> + queue_work(system_freezable_wq,
> >> + &vb->update_balloon_size_work);
> >> + spin_unlock_irqrestore(&vb->stop_update_lock, flags);
> >> + }
> >> +
> >> + virtio_cread(vb->vdev, struct virtio_balloon_config,
> >> + free_page_report_cmd_id, &cmd_id);
> >> + if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
> >> + WRITE_ONCE(vb->report_free_page, false);
> >> + } else if (cmd_id != vb->start_cmd_id) {
> >> + /*
> >> + * Host requests to start the reporting by sending a new cmd
> >> + * id.
> >> + */
> >> + WRITE_ONCE(vb->report_free_page, true);
> >> + vb->start_cmd_id = cmd_id;
> >> + queue_work(vb->balloon_wq, &vb->report_free_page_work);
> >> + }
> >> +}
> >> +
> >> static void update_balloon_size(struct virtio_balloon *vb)
> >> {
> >> u32 actual = vb->num_pages;
> >> @@ -417,40 +513,113 @@ static void update_balloon_size_func(struct
> >> work_struct *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;
> >> + struct scatterlist sg;
> >> + int i, nvqs, err = -ENOMEM;
> >> +
> >> + /* 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_FREE_PAGE_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 = virtio_find_vqs(vb->vdev, nvqs, vqs, callbacks, names, NULL);
> >> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
> >> + callbacks[i] = NULL;
> >> + names[i] = "free_page_vq";
> >> + }
> >> +
> >> + err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names,
> >> + NULL, 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();
> >> + < 0) {
> >> + dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n",
> >> + __func__);
> >> + goto err_find;
> >> + }
> >> virtqueue_kick(vb->stats_vq);
> >> }
> >> +
> >> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ))
> >> + vb->free_page_vq = vqs[i];
> >> +
> >> + kfree(names);
> >> + kfree(callbacks);
> >> + kfree(vqs);
> >> return 0;
> >
> > We can assign err=0 and remove above duplicate code?
> >
>
> Where do you want to assign err=0? Could you show it using code?
o.k you have initialize "err = -ENOMEM;"
Remove these four lines.
- kfree(names);
- kfree(callbacks);
- kfree(vqs);
- return 0;
+ err = 0; // if executed without any error
Below code is already there, so for error, err is already 'ENOMEM'
and a jump to any label.
+
+err_find:
+ kfree(names);
+err_names:
+ kfree(callbacks);
+err_callback:
+ kfree(vqs);
> +err_vq:
> + return err;
> +}
Thanks,
Pankaj
>
>
> Best,
> Wei
>
>
>
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Wei Wang @ 2018-01-17 9:00 UTC (permalink / raw)
To: Pankaj Gupta
Cc: yang zhang wz, virtio-dev, riel, quan xu0, kvm, mst, nilal,
liliang opensource, linux-kernel, virtualization, linux-mm,
pbonzini, akpm, mhocko
In-Reply-To: <1003745745.1007975.1516177271163.JavaMail.zimbra@redhat.com>
On 01/17/2018 04:21 PM, Pankaj Gupta wrote:
>> Negotiation of the VIRTIO_BALLOON_F_FREE_PAGE_VQ feature indicates the
>> support of reporting hints of guest free pages to host via virtio-balloon.
>>
>> Host requests the guest to report free pages by sending a new cmd
>> id to the guest via the free_page_report_cmd_id configuration register.
>>
>> When the guest starts to report, the first element added to the free page
>> vq is the cmd id given by host. When the guest finishes the reporting
>> of all the free pages, VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID is added
>> to the vq to tell host that the reporting is done. Host may also requests
>> the guest to stop the reporting in advance by sending the stop cmd id to
>> the guest via the configuration register.
>>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>> Signed-off-by: Liang Li <liang.z.li@intel.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Michal Hocko <mhocko@kernel.org>
>> ---
>> drivers/virtio/virtio_balloon.c | 242
>> +++++++++++++++++++++++++++++++-----
>> include/uapi/linux/virtio_balloon.h | 4 +
>> 2 files changed, 214 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_balloon.c
>> b/drivers/virtio/virtio_balloon.c
>> index a1fb52c..b9561a5 100644
>> --- a/drivers/virtio/virtio_balloon.c
>> +++ b/drivers/virtio/virtio_balloon.c
>> @@ -53,7 +53,12 @@ static struct vfsmount *balloon_mnt;
>>
>> struct virtio_balloon {
>> struct virtio_device *vdev;
>> - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
>> + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq;
>> +
>> + /* Balloon's own wq for cpu-intensive work items */
>> + struct workqueue_struct *balloon_wq;
>> + /* The free page reporting work item submitted to the balloon wq */
>> + struct work_struct report_free_page_work;
>>
>> /* The balloon servicing is delegated to a freezable workqueue. */
>> struct work_struct update_balloon_stats_work;
>> @@ -63,6 +68,13 @@ struct virtio_balloon {
>> spinlock_t stop_update_lock;
>> bool stop_update;
>>
>> + /* Start to report free pages */
>> + bool report_free_page;
>> + /* Stores the cmd id given by host to start the free page reporting */
>> + uint32_t start_cmd_id;
>> + /* Stores STOP_ID as a sign to tell host that the reporting is done */
>> + uint32_t stop_cmd_id;
>> +
>> /* Waiting for host to ack the pages we released. */
>> wait_queue_head_t acked;
>>
>> @@ -281,6 +293,71 @@ static unsigned int update_balloon_stats(struct
>> virtio_balloon *vb)
>> return idx;
>> }
>>
>> +static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t
>> len)
>> +{
>> + struct scatterlist sg;
>> + unsigned int unused;
>> + int err;
>> +
>> + sg_init_table(&sg, 1);
>> + sg_set_page(&sg, pfn_to_page(pfn), len, 0);
>> +
>> + /* Detach all the used buffers from the vq */
>> + while (virtqueue_get_buf(vq, &unused))
>> + ;
>> +
>> + /*
>> + * Since this is an optimization feature, losing a couple of free
>> + * pages to report isn't important. We simply resturn without adding
>> + * the page if the vq is full. We are adding one entry each time,
>> + * which essentially results in no memory allocation, so the
>> + * GFP_KERNEL flag below can be ignored.
>> + */
>> + if (vq->num_free) {
>> + err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL);
>> + /*
>> + * This is expected to never fail, because there is always an
>> + * entry available on the vq.
>> + */
>> + BUG_ON(err);
>> + }
>> +}
>> +
>> +static void batch_free_page_sg(struct virtqueue *vq,
>> + unsigned long pfn,
>> + uint32_t len)
>> +{
>> + add_one_sg(vq, pfn, len);
>> +
>> + /* Batch till the vq is full */
>> + if (!vq->num_free)
>> + virtqueue_kick(vq);
>> +}
>> +
>> +static void send_cmd_id(struct virtqueue *vq, void *addr)
>> +{
>> + struct scatterlist sg;
>> + unsigned int unused;
>> + int err;
>> +
>> + sg_init_one(&sg, addr, sizeof(uint32_t));
>> +
>> + /*
>> + * This handles the cornercase that the vq happens to be full when
>> + * adding a cmd id. Rarely happen in practice.
>> + */
>> + while (!vq->num_free)
>> + virtqueue_get_buf(vq, &unused);
>> +
>> + err = virtqueue_add_outbuf(vq, &sg, 1, vq, GFP_KERNEL);
>> + /*
>> + * This is expected to never fail, because there is always an
>> + * entry available on the vq.
>> + */
>> + BUG_ON(err);
>> + virtqueue_kick(vq);
>> +}
>> +
>> /*
>> * While most virtqueues communicate guest-initiated requests to the
>> hypervisor,
>> * the stats queue operates in reverse. The driver initializes the
>> virtqueue
>> @@ -316,17 +393,6 @@ static void stats_handle_request(struct virtio_balloon
>> *vb)
>> virtqueue_kick(vq);
>> }
>>
>> -static void virtballoon_changed(struct virtio_device *vdev)
>> -{
>> - struct virtio_balloon *vb = vdev->priv;
>> - unsigned long flags;
>> -
>> - spin_lock_irqsave(&vb->stop_update_lock, flags);
>> - if (!vb->stop_update)
>> - queue_work(system_freezable_wq, &vb->update_balloon_size_work);
>> - spin_unlock_irqrestore(&vb->stop_update_lock, flags);
>> -}
>> -
>> static inline s64 towards_target(struct virtio_balloon *vb)
>> {
>> s64 target;
>> @@ -343,6 +409,36 @@ static inline s64 towards_target(struct virtio_balloon
>> *vb)
>> return target - vb->num_pages;
>> }
>>
>> +static void virtballoon_changed(struct virtio_device *vdev)
>> +{
>> + struct virtio_balloon *vb = vdev->priv;
>> + unsigned long flags;
>> + __u32 cmd_id;
>> + s64 diff = towards_target(vb);
>> +
>> + if (diff) {
>> + spin_lock_irqsave(&vb->stop_update_lock, flags);
>> + if (!vb->stop_update)
>> + queue_work(system_freezable_wq,
>> + &vb->update_balloon_size_work);
>> + spin_unlock_irqrestore(&vb->stop_update_lock, flags);
>> + }
>> +
>> + virtio_cread(vb->vdev, struct virtio_balloon_config,
>> + free_page_report_cmd_id, &cmd_id);
>> + if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
>> + WRITE_ONCE(vb->report_free_page, false);
>> + } else if (cmd_id != vb->start_cmd_id) {
>> + /*
>> + * Host requests to start the reporting by sending a new cmd
>> + * id.
>> + */
>> + WRITE_ONCE(vb->report_free_page, true);
>> + vb->start_cmd_id = cmd_id;
>> + queue_work(vb->balloon_wq, &vb->report_free_page_work);
>> + }
>> +}
>> +
>> static void update_balloon_size(struct virtio_balloon *vb)
>> {
>> u32 actual = vb->num_pages;
>> @@ -417,40 +513,113 @@ static void update_balloon_size_func(struct
>> work_struct *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;
>> + struct scatterlist sg;
>> + int i, nvqs, err = -ENOMEM;
>> +
>> + /* 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_FREE_PAGE_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 = virtio_find_vqs(vb->vdev, nvqs, vqs, callbacks, names, NULL);
>> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
>> + callbacks[i] = NULL;
>> + names[i] = "free_page_vq";
>> + }
>> +
>> + err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names,
>> + NULL, 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();
>> + < 0) {
>> + dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n",
>> + __func__);
>> + goto err_find;
>> + }
>> virtqueue_kick(vb->stats_vq);
>> }
>> +
>> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ))
>> + vb->free_page_vq = vqs[i];
>> +
>> + kfree(names);
>> + kfree(callbacks);
>> + kfree(vqs);
>> return 0;
>
> We can assign err=0 and remove above duplicate code?
>
Where do you want to assign err=0? Could you show it using code?
Best,
Wei
^ permalink raw reply
* Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Pankaj Gupta @ 2018-01-17 8:21 UTC (permalink / raw)
To: Wei Wang
Cc: yang zhang wz, virtio-dev, riel, quan xu0, kvm, mst, nilal,
liliang opensource, linux-kernel, virtualization, linux-mm,
pbonzini, akpm, mhocko
In-Reply-To: <1516165812-3995-3-git-send-email-wei.w.wang@intel.com>
>
> Negotiation of the VIRTIO_BALLOON_F_FREE_PAGE_VQ feature indicates the
> support of reporting hints of guest free pages to host via virtio-balloon.
>
> Host requests the guest to report free pages by sending a new cmd
> id to the guest via the free_page_report_cmd_id configuration register.
>
> When the guest starts to report, the first element added to the free page
> vq is the cmd id given by host. When the guest finishes the reporting
> of all the free pages, VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID is added
> to the vq to tell host that the reporting is done. Host may also requests
> the guest to stop the reporting in advance by sending the stop cmd id to
> the guest via the configuration register.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> ---
> drivers/virtio/virtio_balloon.c | 242
> +++++++++++++++++++++++++++++++-----
> include/uapi/linux/virtio_balloon.h | 4 +
> 2 files changed, 214 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c
> b/drivers/virtio/virtio_balloon.c
> index a1fb52c..b9561a5 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -53,7 +53,12 @@ static struct vfsmount *balloon_mnt;
>
> struct virtio_balloon {
> struct virtio_device *vdev;
> - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq;
> +
> + /* Balloon's own wq for cpu-intensive work items */
> + struct workqueue_struct *balloon_wq;
> + /* The free page reporting work item submitted to the balloon wq */
> + struct work_struct report_free_page_work;
>
> /* The balloon servicing is delegated to a freezable workqueue. */
> struct work_struct update_balloon_stats_work;
> @@ -63,6 +68,13 @@ struct virtio_balloon {
> spinlock_t stop_update_lock;
> bool stop_update;
>
> + /* Start to report free pages */
> + bool report_free_page;
> + /* Stores the cmd id given by host to start the free page reporting */
> + uint32_t start_cmd_id;
> + /* Stores STOP_ID as a sign to tell host that the reporting is done */
> + uint32_t stop_cmd_id;
> +
> /* Waiting for host to ack the pages we released. */
> wait_queue_head_t acked;
>
> @@ -281,6 +293,71 @@ static unsigned int update_balloon_stats(struct
> virtio_balloon *vb)
> return idx;
> }
>
> +static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t
> len)
> +{
> + struct scatterlist sg;
> + unsigned int unused;
> + int err;
> +
> + sg_init_table(&sg, 1);
> + sg_set_page(&sg, pfn_to_page(pfn), len, 0);
> +
> + /* Detach all the used buffers from the vq */
> + while (virtqueue_get_buf(vq, &unused))
> + ;
> +
> + /*
> + * Since this is an optimization feature, losing a couple of free
> + * pages to report isn't important. We simply resturn without adding
> + * the page if the vq is full. We are adding one entry each time,
> + * which essentially results in no memory allocation, so the
> + * GFP_KERNEL flag below can be ignored.
> + */
> + if (vq->num_free) {
> + err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL);
> + /*
> + * This is expected to never fail, because there is always an
> + * entry available on the vq.
> + */
> + BUG_ON(err);
> + }
> +}
> +
> +static void batch_free_page_sg(struct virtqueue *vq,
> + unsigned long pfn,
> + uint32_t len)
> +{
> + add_one_sg(vq, pfn, len);
> +
> + /* Batch till the vq is full */
> + if (!vq->num_free)
> + virtqueue_kick(vq);
> +}
> +
> +static void send_cmd_id(struct virtqueue *vq, void *addr)
> +{
> + struct scatterlist sg;
> + unsigned int unused;
> + int err;
> +
> + sg_init_one(&sg, addr, sizeof(uint32_t));
> +
> + /*
> + * This handles the cornercase that the vq happens to be full when
> + * adding a cmd id. Rarely happen in practice.
> + */
> + while (!vq->num_free)
> + virtqueue_get_buf(vq, &unused);
> +
> + err = virtqueue_add_outbuf(vq, &sg, 1, vq, GFP_KERNEL);
> + /*
> + * This is expected to never fail, because there is always an
> + * entry available on the vq.
> + */
> + BUG_ON(err);
> + virtqueue_kick(vq);
> +}
> +
> /*
> * While most virtqueues communicate guest-initiated requests to the
> hypervisor,
> * the stats queue operates in reverse. The driver initializes the
> virtqueue
> @@ -316,17 +393,6 @@ static void stats_handle_request(struct virtio_balloon
> *vb)
> virtqueue_kick(vq);
> }
>
> -static void virtballoon_changed(struct virtio_device *vdev)
> -{
> - struct virtio_balloon *vb = vdev->priv;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&vb->stop_update_lock, flags);
> - if (!vb->stop_update)
> - queue_work(system_freezable_wq, &vb->update_balloon_size_work);
> - spin_unlock_irqrestore(&vb->stop_update_lock, flags);
> -}
> -
> static inline s64 towards_target(struct virtio_balloon *vb)
> {
> s64 target;
> @@ -343,6 +409,36 @@ static inline s64 towards_target(struct virtio_balloon
> *vb)
> return target - vb->num_pages;
> }
>
> +static void virtballoon_changed(struct virtio_device *vdev)
> +{
> + struct virtio_balloon *vb = vdev->priv;
> + unsigned long flags;
> + __u32 cmd_id;
> + s64 diff = towards_target(vb);
> +
> + if (diff) {
> + spin_lock_irqsave(&vb->stop_update_lock, flags);
> + if (!vb->stop_update)
> + queue_work(system_freezable_wq,
> + &vb->update_balloon_size_work);
> + spin_unlock_irqrestore(&vb->stop_update_lock, flags);
> + }
> +
> + virtio_cread(vb->vdev, struct virtio_balloon_config,
> + free_page_report_cmd_id, &cmd_id);
> + if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
> + WRITE_ONCE(vb->report_free_page, false);
> + } else if (cmd_id != vb->start_cmd_id) {
> + /*
> + * Host requests to start the reporting by sending a new cmd
> + * id.
> + */
> + WRITE_ONCE(vb->report_free_page, true);
> + vb->start_cmd_id = cmd_id;
> + queue_work(vb->balloon_wq, &vb->report_free_page_work);
> + }
> +}
> +
> static void update_balloon_size(struct virtio_balloon *vb)
> {
> u32 actual = vb->num_pages;
> @@ -417,40 +513,113 @@ static void update_balloon_size_func(struct
> work_struct *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;
> + struct scatterlist sg;
> + int i, nvqs, err = -ENOMEM;
> +
> + /* 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_FREE_PAGE_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 = virtio_find_vqs(vb->vdev, nvqs, vqs, callbacks, names, NULL);
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
> + callbacks[i] = NULL;
> + names[i] = "free_page_vq";
> + }
> +
> + err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names,
> + NULL, 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();
> + < 0) {
> + dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n",
> + __func__);
> + goto err_find;
> + }
> virtqueue_kick(vb->stats_vq);
> }
> +
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ))
> + vb->free_page_vq = vqs[i];
> +
> + kfree(names);
> + kfree(callbacks);
> + kfree(vqs);
> return 0;
We can assign err=0 and remove above duplicate code?
> +
> +err_find:
> + kfree(names);
> +err_names:
> + kfree(callbacks);
> +err_callback:
> + kfree(vqs);
> +err_vq:
> + return err;
> +}
> +
> +static bool virtio_balloon_send_free_pages(void *opaque, unsigned long pfn,
> + unsigned long nr_pages)
> +{
> + struct virtio_balloon *vb = (struct virtio_balloon *)opaque;
> + uint32_t len = nr_pages << PAGE_SHIFT;
> +
> + if (!READ_ONCE(vb->report_free_page))
> + return false;
> +
> + batch_free_page_sg(vb->free_page_vq, pfn, len);
> +
> + return true;
> +}
> +
> +static void report_free_page_func(struct work_struct *work)
> +{
> + struct virtio_balloon *vb;
> +
> + vb = container_of(work, struct virtio_balloon, report_free_page_work);
> + /* Start by sending the obtained cmd id to the host with an outbuf */
> + send_cmd_id(vb->free_page_vq, &vb->start_cmd_id);
> + walk_free_mem_block(vb, 0, &virtio_balloon_send_free_pages);
> + /* End by sending the stop id to the host with an outbuf */
> + send_cmd_id(vb->free_page_vq, &vb->stop_cmd_id);
> }
>
> #ifdef CONFIG_BALLOON_COMPACTION
> @@ -566,6 +735,13 @@ static int virtballoon_probe(struct virtio_device *vdev)
> if (err)
> goto out_free_vb;
>
> + if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
> + vb->balloon_wq = alloc_workqueue("balloon-wq",
> + WQ_FREEZABLE | WQ_CPU_INTENSIVE, 0);
> + INIT_WORK(&vb->report_free_page_work, report_free_page_func);
> + vb->stop_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID;
> + }
> +
> vb->nb.notifier_call = virtballoon_oom_notify;
> vb->nb.priority = VIRTBALLOON_OOM_NOTIFY_PRIORITY;
> err = register_oom_notifier(&vb->nb);
> @@ -630,6 +806,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->report_free_page_work);
>
> remove_common(vb);
> #ifdef CONFIG_BALLOON_COMPACTION
> @@ -682,6 +859,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_FREE_PAGE_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 343d7dd..55e2456 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -34,15 +34,19 @@
> #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_FREE_PAGE_VQ 3 /* VQ to report free pages */
>
> /* Size of a PFN in the balloon interface. */
> #define VIRTIO_BALLOON_PFN_SHIFT 12
>
> +#define VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID 0
> struct virtio_balloon_config {
> /* Number of pages host wants Guest to give up. */
> __u32 num_pages;
> /* Number of pages we've actually got in balloon. */
> __u32 actual;
> + /* Free page report command id, readonly by guest */
> + __u32 free_page_report_cmd_id;
> };
>
> #define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
> --
> 2.7.4
>
>
^ permalink raw reply
* [PATCH v2 net-next] virtio_net: Add ethtool stats
From: Toshiaki Makita @ 2018-01-17 6:38 UTC (permalink / raw)
To: David S . Miller, Michael S . Tsirkin, Jason Wang
Cc: netdev, virtualization, Florian Fainelli
The main purpose of this patch is adding a way of checking per-queue stats.
It's useful to debug performance problems on multiqueue environment.
$ ethtool -S ens10
NIC statistics:
rx_queue_0_packets: 2090408
rx_queue_0_bytes: 3164825094
rx_queue_1_packets: 2082531
rx_queue_1_bytes: 3152932314
tx_queue_0_packets: 2770841
tx_queue_0_bytes: 4194955474
tx_queue_1_packets: 3084697
tx_queue_1_bytes: 4670196372
This change converts existing per-cpu stats structure into per-queue one.
This should not impact on performance since each queue counter is not
updated concurrently by multiple cpus.
Performance numbers:
- Guest has 2 vcpus and 2 queues
- Guest runs netserver
- Host runs 100-flow super_netperf
Before After Diff
UDP_STREAM 18byte 86.22 87.00 +0.90%
UDP_STREAM 1472byte 4055.27 4042.18 -0.32%
TCP_STREAM 16956.32 16890.63 -0.39%
UDP_RR 178667.11 185862.70 +4.03%
TCP_RR 128473.04 124985.81 -2.71%
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
v2:
- Removed redundant counters which can be obtained from dev_get_stats.
- Made queue counter structure different for tx and rx so they can be
easily extended separately, as some additional counters are expected
like XDP related ones and VM-Exit event.
- Added performance numbers in commitlog.
drivers/net/virtio_net.c | 191 ++++++++++++++++++++++++++++++++++-------------
1 file changed, 141 insertions(+), 50 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 12dfc5f..626c273 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -66,16 +66,39 @@
VIRTIO_NET_F_GUEST_UFO
};
-struct virtnet_stats {
- struct u64_stats_sync tx_syncp;
- struct u64_stats_sync rx_syncp;
- u64 tx_bytes;
- u64 tx_packets;
-
- u64 rx_bytes;
- u64 rx_packets;
+struct virtnet_stat_desc {
+ char desc[ETH_GSTRING_LEN];
+ size_t offset;
};
+struct virtnet_sq_stats {
+ struct u64_stats_sync syncp;
+ u64 packets;
+ u64 bytes;
+};
+
+struct virtnet_rq_stats {
+ struct u64_stats_sync syncp;
+ u64 packets;
+ u64 bytes;
+};
+
+#define VIRTNET_SQ_STAT(m) offsetof(struct virtnet_sq_stats, m)
+#define VIRTNET_RQ_STAT(m) offsetof(struct virtnet_rq_stats, m)
+
+static const struct virtnet_stat_desc virtnet_sq_stats_desc[] = {
+ { "packets", VIRTNET_SQ_STAT(packets) },
+ { "bytes", VIRTNET_SQ_STAT(bytes) },
+};
+
+static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = {
+ { "packets", VIRTNET_RQ_STAT(packets) },
+ { "bytes", VIRTNET_RQ_STAT(bytes) },
+};
+
+#define VIRTNET_SQ_STATS_LEN ARRAY_SIZE(virtnet_sq_stats_desc)
+#define VIRTNET_RQ_STATS_LEN ARRAY_SIZE(virtnet_rq_stats_desc)
+
/* Internal representation of a send virtqueue */
struct send_queue {
/* Virtqueue associated with this send _queue */
@@ -87,6 +110,8 @@ struct send_queue {
/* Name of the send queue: output.$index */
char name[40];
+ struct virtnet_sq_stats stats;
+
struct napi_struct napi;
};
@@ -99,6 +124,8 @@ struct receive_queue {
struct bpf_prog __rcu *xdp_prog;
+ struct virtnet_rq_stats stats;
+
/* Chain pages by the private ptr. */
struct page *pages;
@@ -152,9 +179,6 @@ struct virtnet_info {
/* Packet virtio header size */
u8 hdr_len;
- /* Active statistics */
- struct virtnet_stats __percpu *stats;
-
/* Work struct for refilling if we run low on memory. */
struct delayed_work refill;
@@ -1127,7 +1151,6 @@ static int virtnet_receive(struct receive_queue *rq, int budget, bool *xdp_xmit)
struct virtnet_info *vi = rq->vq->vdev->priv;
unsigned int len, received = 0, bytes = 0;
void *buf;
- struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
if (!vi->big_packets || vi->mergeable_rx_bufs) {
void *ctx;
@@ -1150,10 +1173,10 @@ static int virtnet_receive(struct receive_queue *rq, int budget, bool *xdp_xmit)
schedule_delayed_work(&vi->refill, 0);
}
- u64_stats_update_begin(&stats->rx_syncp);
- stats->rx_bytes += bytes;
- stats->rx_packets += received;
- u64_stats_update_end(&stats->rx_syncp);
+ u64_stats_update_begin(&rq->stats.syncp);
+ rq->stats.bytes += bytes;
+ rq->stats.packets += received;
+ u64_stats_update_end(&rq->stats.syncp);
return received;
}
@@ -1162,8 +1185,6 @@ static void free_old_xmit_skbs(struct send_queue *sq)
{
struct sk_buff *skb;
unsigned int len;
- struct virtnet_info *vi = sq->vq->vdev->priv;
- struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
unsigned int packets = 0;
unsigned int bytes = 0;
@@ -1182,10 +1203,10 @@ static void free_old_xmit_skbs(struct send_queue *sq)
if (!packets)
return;
- u64_stats_update_begin(&stats->tx_syncp);
- stats->tx_bytes += bytes;
- stats->tx_packets += packets;
- u64_stats_update_end(&stats->tx_syncp);
+ u64_stats_update_begin(&sq->stats.syncp);
+ sq->stats.bytes += bytes;
+ sq->stats.packets += packets;
+ u64_stats_update_end(&sq->stats.syncp);
}
static void virtnet_poll_cleantx(struct receive_queue *rq)
@@ -1474,24 +1495,25 @@ static void virtnet_stats(struct net_device *dev,
struct rtnl_link_stats64 *tot)
{
struct virtnet_info *vi = netdev_priv(dev);
- int cpu;
unsigned int start;
+ int i;
- for_each_possible_cpu(cpu) {
- struct virtnet_stats *stats = per_cpu_ptr(vi->stats, cpu);
+ for (i = 0; i < vi->max_queue_pairs; i++) {
u64 tpackets, tbytes, rpackets, rbytes;
+ struct receive_queue *rq = &vi->rq[i];
+ struct send_queue *sq = &vi->sq[i];
do {
- start = u64_stats_fetch_begin_irq(&stats->tx_syncp);
- tpackets = stats->tx_packets;
- tbytes = stats->tx_bytes;
- } while (u64_stats_fetch_retry_irq(&stats->tx_syncp, start));
+ start = u64_stats_fetch_begin_irq(&sq->stats.syncp);
+ tpackets = sq->stats.packets;
+ tbytes = sq->stats.bytes;
+ } while (u64_stats_fetch_retry_irq(&sq->stats.syncp, start));
do {
- start = u64_stats_fetch_begin_irq(&stats->rx_syncp);
- rpackets = stats->rx_packets;
- rbytes = stats->rx_bytes;
- } while (u64_stats_fetch_retry_irq(&stats->rx_syncp, start));
+ start = u64_stats_fetch_begin_irq(&rq->stats.syncp);
+ rpackets = rq->stats.packets;
+ rbytes = rq->stats.bytes;
+ } while (u64_stats_fetch_retry_irq(&rq->stats.syncp, start));
tot->rx_packets += rpackets;
tot->tx_packets += tpackets;
@@ -1829,6 +1851,83 @@ static int virtnet_set_channels(struct net_device *dev,
return err;
}
+static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ char *p = (char *)data;
+ unsigned int i, j;
+
+ switch (stringset) {
+ case ETH_SS_STATS:
+ for (i = 0; i < vi->curr_queue_pairs; i++) {
+ for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++) {
+ snprintf(p, ETH_GSTRING_LEN, "rx_queue_%u_%s",
+ i, virtnet_rq_stats_desc[j].desc);
+ p += ETH_GSTRING_LEN;
+ }
+ }
+
+ for (i = 0; i < vi->curr_queue_pairs; i++) {
+ for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++) {
+ snprintf(p, ETH_GSTRING_LEN, "tx_queue_%u_%s",
+ i, virtnet_sq_stats_desc[j].desc);
+ p += ETH_GSTRING_LEN;
+ }
+ }
+ break;
+ }
+}
+
+static int virtnet_get_sset_count(struct net_device *dev, int sset)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ switch (sset) {
+ case ETH_SS_STATS:
+ return vi->curr_queue_pairs * (VIRTNET_RQ_STATS_LEN +
+ VIRTNET_SQ_STATS_LEN);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void virtnet_get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ unsigned int idx = 0, start, i, j;
+ const u8 *stats_base;
+ size_t offset;
+
+ for (i = 0; i < vi->curr_queue_pairs; i++) {
+ struct receive_queue *rq = &vi->rq[i];
+
+ stats_base = (u8 *)&rq->stats;
+ do {
+ start = u64_stats_fetch_begin_irq(&rq->stats.syncp);
+ for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++) {
+ offset = virtnet_rq_stats_desc[j].offset;
+ data[idx + j] = *(u64 *)(stats_base + offset);
+ }
+ } while (u64_stats_fetch_retry_irq(&rq->stats.syncp, start));
+ idx += VIRTNET_RQ_STATS_LEN;
+ }
+
+ for (i = 0; i < vi->curr_queue_pairs; i++) {
+ struct send_queue *sq = &vi->sq[i];
+
+ stats_base = (u8 *)&sq->stats;
+ do {
+ start = u64_stats_fetch_begin_irq(&sq->stats.syncp);
+ for (j = 0; j < VIRTNET_SQ_STATS_LEN; j++) {
+ offset = virtnet_sq_stats_desc[j].offset;
+ data[idx + j] = *(u64 *)(stats_base + offset);
+ }
+ } while (u64_stats_fetch_retry_irq(&sq->stats.syncp, start));
+ idx += VIRTNET_SQ_STATS_LEN;
+ }
+}
+
static void virtnet_get_channels(struct net_device *dev,
struct ethtool_channels *channels)
{
@@ -1928,6 +2027,9 @@ static void virtnet_update_settings(struct virtnet_info *vi)
.get_drvinfo = virtnet_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ringparam = virtnet_get_ringparam,
+ .get_strings = virtnet_get_strings,
+ .get_sset_count = virtnet_get_sset_count,
+ .get_ethtool_stats = virtnet_get_ethtool_stats,
.set_channels = virtnet_set_channels,
.get_channels = virtnet_get_channels,
.get_ts_info = ethtool_op_get_ts_info,
@@ -2420,6 +2522,9 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
ewma_pkt_len_init(&vi->rq[i].mrg_avg_pkt_len);
sg_init_table(vi->sq[i].sg, ARRAY_SIZE(vi->sq[i].sg));
+
+ u64_stats_init(&vi->rq[i].stats.syncp);
+ u64_stats_init(&vi->sq[i].stats.syncp);
}
return 0;
@@ -2544,7 +2649,7 @@ static int virtnet_validate(struct virtio_device *vdev)
static int virtnet_probe(struct virtio_device *vdev)
{
- int i, err;
+ int i, err = -ENOMEM;
struct net_device *dev;
struct virtnet_info *vi;
u16 max_queue_pairs;
@@ -2621,17 +2726,6 @@ static int virtnet_probe(struct virtio_device *vdev)
vi->dev = dev;
vi->vdev = vdev;
vdev->priv = vi;
- vi->stats = alloc_percpu(struct virtnet_stats);
- err = -ENOMEM;
- if (vi->stats == NULL)
- goto free;
-
- for_each_possible_cpu(i) {
- struct virtnet_stats *virtnet_stats;
- virtnet_stats = per_cpu_ptr(vi->stats, i);
- u64_stats_init(&virtnet_stats->tx_syncp);
- u64_stats_init(&virtnet_stats->rx_syncp);
- }
INIT_WORK(&vi->config_work, virtnet_config_changed_work);
@@ -2668,7 +2762,7 @@ static int virtnet_probe(struct virtio_device *vdev)
*/
dev_err(&vdev->dev, "device MTU appears to have changed "
"it is now %d < %d", mtu, dev->min_mtu);
- goto free_stats;
+ goto free;
}
dev->mtu = mtu;
@@ -2692,7 +2786,7 @@ static int virtnet_probe(struct virtio_device *vdev)
/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
err = init_vqs(vi);
if (err)
- goto free_stats;
+ goto free;
#ifdef CONFIG_SYSFS
if (vi->mergeable_rx_bufs)
@@ -2747,8 +2841,6 @@ static int virtnet_probe(struct virtio_device *vdev)
cancel_delayed_work_sync(&vi->refill);
free_receive_page_frags(vi);
virtnet_del_vqs(vi);
-free_stats:
- free_percpu(vi->stats);
free:
free_netdev(dev);
return err;
@@ -2781,7 +2873,6 @@ static void virtnet_remove(struct virtio_device *vdev)
remove_vq_common(vi);
- free_percpu(vi->stats);
free_netdev(vi->dev);
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v22 3/3] virtio-balloon: don't report free pages when page poisoning is enabled
From: Wei Wang @ 2018-01-17 5:10 UTC (permalink / raw)
To: virtio-dev, linux-kernel, virtualization, kvm, linux-mm, mst,
mhocko, akpm
Cc: yang.zhang.wz, riel, quan.xu0, liliang.opensource, pbonzini,
nilal
In-Reply-To: <1516165812-3995-1-git-send-email-wei.w.wang@intel.com>
The guest free pages should not be discarded by the live migration thread
when page poisoning is enabled with PAGE_POISONING_NO_SANITY=n, because
skipping the transfer of such poisoned free pages will trigger false
positive when new pages are allocated and checked on the destination.
This patch adds a config field, poison_val. Guest writes to the config
field to tell the host about the poisoning value. The value will be 0 in
the following cases:
1) PAGE_POISONING_NO_SANITY is enabled;
2) page poisoning is disabled; or
3) PAGE_POISONING_ZERO is enabled.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Cc: Michal Hocko <mhocko@suse.com>
---
drivers/virtio/virtio_balloon.c | 8 ++++++++
include/uapi/linux/virtio_balloon.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index b9561a5..5a42235 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -706,6 +706,7 @@ static struct file_system_type balloon_fs = {
static int virtballoon_probe(struct virtio_device *vdev)
{
struct virtio_balloon *vb;
+ __u32 poison_val;
int err;
if (!vdev->config->get) {
@@ -740,6 +741,13 @@ static int virtballoon_probe(struct virtio_device *vdev)
WQ_FREEZABLE | WQ_CPU_INTENSIVE, 0);
INIT_WORK(&vb->report_free_page_work, report_free_page_func);
vb->stop_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID;
+ if (IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY) ||
+ !page_poisoning_enabled())
+ poison_val = 0;
+ else
+ poison_val = PAGE_POISON;
+ virtio_cwrite(vb->vdev, struct virtio_balloon_config,
+ poison_val, &poison_val);
}
vb->nb.notifier_call = virtballoon_oom_notify;
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 55e2456..5861876 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -47,6 +47,8 @@ struct virtio_balloon_config {
__u32 actual;
/* Free page report command id, readonly by guest */
__u32 free_page_report_cmd_id;
+ /* Stores PAGE_POISON if page poisoning with sanity check is in use */
+ __u32 poison_val;
};
#define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
--
2.7.4
^ permalink raw reply related
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