[PATCH v2] virtio_balloon: Fix endless deflation and inflation on arm64
The deflation request to the target, which isn't unaligned to the
guest page size causes endless deflation and inflation actions. For
example, we receive the flooding QMP events for the changes on memory
balloon's size after a deflation request to the unaligned target is
sent for the ARM64 guest, where we have 64KB base page size.
/home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
-accel kvm -machine virt,gic-version=host -cpu host \
-smp maxcpus=8,cpus=8,sockets=2,clusters=2,cores=2,threads=1 \
-m 1024M,slots=16,maxmem=64G \
-object memory-backend-ram,id=mem0,size=512M \
-object memory-backend-ram,id=mem1,size=512M \
-numa node,nodeid=0,memdev=mem0,cpus=0-3 \
-numa node,nodeid=1,memdev=mem1,cpus=4-7 \
: \
-device virtio-balloon-pci,id=balloon0,bus=pcie.10
{ "execute" : "balloon", "arguments": { "value" : 1073672192 } }
{"return": {}}
{"timestamp": {"seconds": 1693272173, "microseconds": 88667}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272174, "microseconds": 89704}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272175, "microseconds": 90819}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272176, "microseconds": 91961}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272177, "microseconds": 93040}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272178, "microseconds": 94117}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272179, "microseconds": 95337}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272180, "microseconds": 96615}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272181, "microseconds": 97626}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272182, "microseconds": 98693}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272183, "microseconds": 99698}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272184, "microseconds": 100727}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272185, "microseconds": 90430}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272186, "microseconds": 102999}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
:
Fix it by aligning the target up to the guest page size, 64KB in this
specific case. With this applied, no flooding QMP events are observed
and the memory balloon's size can be stablizied to 0x3ffe soon
after the deflation request is sent.
{ "execute" : "balloon", "arguments": { "value" : 1073672192 } }
{"return": {}}
{"timestamp": {"seconds": 1693273328, "microseconds": 793075}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{ "execute" : "query-balloon" }
{"return": {"actual": 1073610752}}
Signed-off-by: Gavin Shan
Tested-by: Zhenyu Zhang
---
v2: Align @num_pages up to the guest page size in towards_target()
directly as David suggested.
---
drivers/virtio/virtio_balloon.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 5b15936a5214..2d5d252ef419 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -395,7 +395,11 @@ static inline s64 towards_target(struct virtio_balloon *vb)
virtio_cread_le(vb->vdev, struct virtio_balloon_config, num_pages,
&num_pages);
- target = num_pages;
+ /*
+* Aligned up to guest page size to avoid inflating and deflating
+* balloon endlessly.
+*/
+ target = ALIGN(num_pages, VIRTIO_BALLOON_PAGES_PER_PAGE);
return target - vb->num_pages;
}
--
2.41.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_balloon: Fix endless deflation and inflation on arm64
On 8/31/23 02:30, David Hildenbrand wrote:
On 29.08.23 03:54, Gavin Shan wrote:
The deflation request to the target, which isn't unaligned to the
guest page size causes endless deflation and inflation actions. For
example, we receive the flooding QMP events for the changes on memory
balloon's size after a deflation request to the unaligned target is
sent for the ARM64 guest, where we have 64KB base page size.
/home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
-accel kvm -machine virt,gic-version=host -cpu host \
-smp maxcpus=8,cpus=8,sockets=2,clusters=2,cores=2,threads=1 \
-m 1024M,slots=16,maxmem=64G \
-object memory-backend-ram,id=mem0,size=512M \
-object memory-backend-ram,id=mem1,size=512M \
-numa node,nodeid=0,memdev=mem0,cpus=0-3 \
-numa node,nodeid=1,memdev=mem1,cpus=4-7 \
: \
-device virtio-balloon-pci,id=balloon0,bus=pcie.10
{ "execute" : "balloon", "arguments": { "value" : 1073672192 } }
{"return": {}}
{"timestamp": {"seconds": 1693272173, "microseconds": 88667}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272174, "microseconds": 89704}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272175, "microseconds": 90819}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272176, "microseconds": 91961}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272177, "microseconds": 93040}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272178, "microseconds": 94117}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272179, "microseconds": 95337}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272180, "microseconds": 96615}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272181, "microseconds": 97626}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272182, "microseconds": 98693}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272183, "microseconds": 99698}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272184, "microseconds": 100727}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272185, "microseconds": 90430}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272186, "microseconds": 102999}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
:
Fix it by having the target aligned to the guest page size, 64KB
in this specific case. With this applied, no flooding QMP event
is observed and the memory balloon's size can be stablizied to
0x3ffe soon after the deflation request is sent.
{ "execute" : "balloon", "arguments": { "value" : 1073672192 } }
{"return": {}}
{"timestamp": {"seconds": 1693273328, "microseconds": 793075}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{ "execute" : "query-balloon" }
{"return": {"actual": 1073610752}}
Signed-off-by: Gavin Shan
---
drivers/virtio/virtio_balloon.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 5b15936a5214..625caac35264 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -386,6 +386,17 @@ static void stats_handle_request(struct virtio_balloon *vb)
virtqueue_kick(vq);
}
+static inline s64 align_pages_up(s64 diff)
+{
+ if (diff == 0)
+ return diff;
+
+ if (diff > 0)
+ return ALIGN(diff, VIRTIO_BALLOON_PAGES_PER_PAGE);
+
+ return -ALIGN(-diff, VIRTIO_BALLOON_PAGES_PER_PAGE);
+}
+
static inline s64 towards_target(struct virtio_balloon *vb)
{
s64 target;
@@ -396,7 +407,7 @@ static inline s64 towards_target(struct virtio_balloon *vb)
&num_pages);
target = num_pages;
- return target - vb->num_pages;
We know that vb->num_pages is always multiples of VIRTIO_BALLOON_PAGES_PER_PAGE.
Why not simply align target down?
target = ALIGN(num_pages, VIRTIO_BALLOON_PAGES_PER_PAGE);
return target - vb->num_pages;
Good point. Thanks a lot, David. The code will be changed to what's suggested in
v2, to be posted soon. I will also add a comment to explain it a bit. Besides,
ALIGN()
is align-up instead of align-down to give bias to deflation intentionally, to
avoid
overrunning the machine's memory size if it's not alig
Re: [PATCH net-next V1 0/4] virtio_net: add per queue interrupt coalescing support
On Fri, Jul 14, 2023 at 10:10:51AM +0800, Xuan Zhuo wrote: > On Thu, 13 Jul 2023 07:40:12 -0400, "Michael S. Tsirkin" > wrote: > > On Mon, Jul 10, 2023 at 12:20:01PM +0300, Gavin Li wrote: > > > Currently, coalescing parameters are grouped for all transmit and receive > > > virtqueues. This patch series add support to set or get the parameters for > > > a specified virtqueue. > > > > > > When the traffic between virtqueues is unbalanced, for example, one > > > virtqueue > > > is busy and another virtqueue is idle, then it will be very useful to > > > control coalescing parameters at the virtqueue granularity. > > > > series: > > > > Acked-by: Michael S. Tsirkin > > > Why? > > This series has the bug I reported. > > Are you thinking that is ok? Or this is not a bug? > > Thanks. > > I missed that mail. What's the bug? > > > > > > > > > Example command: > > > $ ethtool -Q eth5 queue_mask 0x1 --coalesce tx-packets 10 > > > Would set max_packets=10 to VQ 1. > > > $ ethtool -Q eth5 queue_mask 0x1 --coalesce rx-packets 10 > > > Would set max_packets=10 to VQ 0. > > > $ ethtool -Q eth5 queue_mask 0x1 --show-coalesce > > > Queue: 0 > > > Adaptive RX: off TX: off > > > stats-block-usecs: 0 > > > sample-interval: 0 > > > pkt-rate-low: 0 > > > pkt-rate-high: 0 > > > > > > rx-usecs: 222 > > > rx-frames: 0 > > > rx-usecs-irq: 0 > > > rx-frames-irq: 256 > > > > > > tx-usecs: 222 > > > tx-frames: 0 > > > tx-usecs-irq: 0 > > > tx-frames-irq: 256 > > > > > > rx-usecs-low: 0 > > > rx-frame-low: 0 > > > tx-usecs-low: 0 > > > tx-frame-low: 0 > > > > > > rx-usecs-high: 0 > > > rx-frame-high: 0 > > > tx-usecs-high: 0 > > > tx-frame-high: 0 > > > > > > In this patch series: > > > Patch-1: Extract interrupt coalescing settings to a structure. > > > Patch-2: Extract get/set interrupt coalesce to a function. > > > Patch-3: Support per queue interrupt coalesce command. > > > Patch-4: Enable per queue interrupt coalesce feature. > > > > > > Gavin Li (4): > > > virtio_net: extract interrupt coalescing settings to a structure > > > virtio_net: extract get/set interrupt coalesce to a function > > > virtio_net: support per queue interrupt coalesce command > > > virtio_net: enable per queue interrupt coalesce feature > > > > > > drivers/net/virtio_net.c| 169 ++-- > > > include/uapi/linux/virtio_net.h | 14 +++ > > > 2 files changed, 154 insertions(+), 29 deletions(-) > > > > > > -- > > > 2.39.1 > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_balloon: Fix endless deflation and inflation on arm64
On 29.08.23 03:54, Gavin Shan wrote:
The deflation request to the target, which isn't unaligned to the
guest page size causes endless deflation and inflation actions. For
example, we receive the flooding QMP events for the changes on memory
balloon's size after a deflation request to the unaligned target is
sent for the ARM64 guest, where we have 64KB base page size.
/home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
-accel kvm -machine virt,gic-version=host -cpu host \
-smp maxcpus=8,cpus=8,sockets=2,clusters=2,cores=2,threads=1 \
-m 1024M,slots=16,maxmem=64G \
-object memory-backend-ram,id=mem0,size=512M \
-object memory-backend-ram,id=mem1,size=512M \
-numa node,nodeid=0,memdev=mem0,cpus=0-3 \
-numa node,nodeid=1,memdev=mem1,cpus=4-7 \
: \
-device virtio-balloon-pci,id=balloon0,bus=pcie.10
{ "execute" : "balloon", "arguments": { "value" : 1073672192 } }
{"return": {}}
{"timestamp": {"seconds": 1693272173, "microseconds": 88667}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272174, "microseconds": 89704}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272175, "microseconds": 90819}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272176, "microseconds": 91961}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272177, "microseconds": 93040}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272178, "microseconds": 94117}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272179, "microseconds": 95337}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272180, "microseconds": 96615}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272181, "microseconds": 97626}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272182, "microseconds": 98693}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
{"timestamp": {"seconds": 1693272183, "microseconds": 99698}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272184, "microseconds": 100727}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272185, "microseconds": 90430}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{"timestamp": {"seconds": 1693272186, "microseconds": 102999}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073676288}}
:
Fix it by having the target aligned to the guest page size, 64KB
in this specific case. With this applied, no flooding QMP event
is observed and the memory balloon's size can be stablizied to
0x3ffe soon after the deflation request is sent.
{ "execute" : "balloon", "arguments": { "value" : 1073672192 } }
{"return": {}}
{"timestamp": {"seconds": 1693273328, "microseconds": 793075}, \
"event": "BALLOON_CHANGE", "data": {"actual": 1073610752}}
{ "execute" : "query-balloon" }
{"return": {"actual": 1073610752}}
Signed-off-by: Gavin Shan
---
drivers/virtio/virtio_balloon.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 5b15936a5214..625caac35264 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -386,6 +386,17 @@ static void stats_handle_request(struct virtio_balloon *vb)
virtqueue_kick(vq);
}
+static inline s64 align_pages_up(s64 diff)
+{
+ if (diff == 0)
+ return diff;
+
+ if (diff > 0)
+ return ALIGN(diff, VIRTIO_BALLOON_PAGES_PER_PAGE);
+
+ return -ALIGN(-diff, VIRTIO_BALLOON_PAGES_PER_PAGE);
+}
+
static inline s64 towards_target(struct virtio_balloon *vb)
{
s64 target;
@@ -396,7 +407,7 @@ static inline s64 towards_target(struct virtio_balloon *vb)
&num_pages);
target = num_pages;
- return target - vb->num_pages;
We know that vb->num_pages is always multiples of
VIRTIO_BALLOON_PAGES_PER_PAGE.
Why not simply align target down?
target = ALIGN(num_pages, VIRTIO_BALLOON_PAGES_PER_PAGE);
return target - vb->num_pages;
--
Cheers,
David / dhildenb
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 0/3] vduse: add support for networking devices
On Wed, Aug 30, 2023 at 01:27:18PM +0200, Maxime Coquelin wrote: > > > On 8/29/23 19:05, Michael S. Tsirkin wrote: > > On Tue, Aug 29, 2023 at 03:34:06PM +0200, Maxime Coquelin wrote: > > > > > > > > > On 8/11/23 00:00, Jakub Kicinski wrote: > > > > On Thu, 10 Aug 2023 17:42:11 -0400 Michael S. Tsirkin wrote: > > > > > > Directly into the stack? I thought VDUSE is vDPA in user space, > > > > > > meaning to get to the kernel the packet has to first go thru > > > > > > a virtio-net instance. > > > > > > > > > > yes. is that a sufficient filter in your opinion? > > > > > > > > Yes, the ability to create the device feels stronger than CAP_NET_RAW, > > > > and a bit tangential to CAP_NET_ADMIN. But I don't have much practical > > > > experience with virt so no strong opinion, perhaps it does make sense > > > > for someone's deployment? Dunno.. > > > > > > > > > > I'm not sure CAP_NET_ADMIN should be required for creating the VDUSE > > > devices, as the device could be attached to vhost-vDPA and so not > > > visible to the Kernel networking stack. > > > > > > However, CAP_NET_ADMIN should be required to attach the VDUSE device to > > > virtio-vdpa/virtio-net. > > > > > > Does that make sense? > > > > > > Maxime > > > > OK. How are we going to enforce it? > > Actually, it seems already enforced for all VDPA devices types. > Indeed, the VDPA_CMD_DEV_NEW Netlink command used to add the device to > the VDPA bus has the GENL_ADMIN_PERM flag set, and so require > CAT_NET_ADMIN. Hmm good point. Pity I didn't notice earlier. Oh well there's always the next release. > > Also, we need a way for selinux to enable/disable some of these things > > but not others. > > Ok, I can do it in a patch on top. > Do you have a pointer where it is done for Virtio Block devices? > > Maxime It's not done yet - at the moment vduse device is always block so we didn't need the distinction. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 0/3] vduse: add support for networking devices
On 8/29/23 19:05, Michael S. Tsirkin wrote: On Tue, Aug 29, 2023 at 03:34:06PM +0200, Maxime Coquelin wrote: On 8/11/23 00:00, Jakub Kicinski wrote: On Thu, 10 Aug 2023 17:42:11 -0400 Michael S. Tsirkin wrote: Directly into the stack? I thought VDUSE is vDPA in user space, meaning to get to the kernel the packet has to first go thru a virtio-net instance. yes. is that a sufficient filter in your opinion? Yes, the ability to create the device feels stronger than CAP_NET_RAW, and a bit tangential to CAP_NET_ADMIN. But I don't have much practical experience with virt so no strong opinion, perhaps it does make sense for someone's deployment? Dunno.. I'm not sure CAP_NET_ADMIN should be required for creating the VDUSE devices, as the device could be attached to vhost-vDPA and so not visible to the Kernel networking stack. However, CAP_NET_ADMIN should be required to attach the VDUSE device to virtio-vdpa/virtio-net. Does that make sense? Maxime OK. How are we going to enforce it? Actually, it seems already enforced for all VDPA devices types. Indeed, the VDPA_CMD_DEV_NEW Netlink command used to add the device to the VDPA bus has the GENL_ADMIN_PERM flag set, and so require CAT_NET_ADMIN. Also, we need a way for selinux to enable/disable some of these things but not others. Ok, I can do it in a patch on top. Do you have a pointer where it is done for Virtio Block devices? Maxime ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
