On 14.12.23 05:13, David Stevens wrote:
On Wed, Dec 13, 2023 at 5:44 PM David Hildenbrand <david@xxxxxxxxxx> wrote:
On 11.12.23 12:43, David Stevens wrote:
From: David Stevens <stevensd@xxxxxxxxxxxx>
Hi David,
Add a wakeup event for when the balloon is inflating or deflating.
Userspace can enable this wakeup event to prevent the system from
suspending while the balloon is being adjusted. This allows
/sys/power/wakeup_count to be used without breaking virtio_balloon's
cooperative memory management.
Can you add/share some more details
I'm working on enabling support for Linux s2Idle in our Android
virtual machine, to restrict apps from running in the background
without holding an Android partial wakelock. With the patch I recently
sent out [1], since crosvm advertises native PCI power management for
virtio devices, the Android guest can properly enter s2idle, and it
can be woken up by incoming IO. However, one of the remaining problems
is that when the host needs to reclaim memory from the guest via the
virtio-balloon, there is nothing preventing the guest from entering
s2idle before the balloon driver finishes returning memory to the
host.
Thanks for the information. So you also want to wakeup the VM when
wanting to get more memory from the VM?
Using which mechanism would that wakeup happen? Likely not the device
itself?
One alternative to this approach would be to add a virtballoon_suspend
callback to abort suspend if the balloon is inflating/adjusting.
However, it seems cleaner to just prevent suspend in the first place.
Also, the PM notifier could also be used with very high priority, so the
device would respond early to PM_SUSPEND_PREPARE.
[...]
queue_work(system_freezable_wq, work);
+ else
+ end_update_balloon_size(vb, seqno);
What if we stop the workqueue and unload the driver -- see
remove_common() -- won't you leave pm_stay_awake() wrongly set?
When a device gets removed, its wakeup source is destroyed, which
automatically calls __pm_relax.
Ah, thanks.
}
static int init_vqs(struct virtio_balloon *vb)
@@ -992,6 +1028,9 @@ static int virtballoon_probe(struct virtio_device *vdev)
goto out_unregister_oom;
}
+ spin_lock_init(&vb->adjustment_lock);
+ device_set_wakeup_capable(&vb->vdev->dev, true);
I'm a bit confused: Documentation/driver-api/pm/devices.rst documents
"
The :c:member:`power.can_wakeup` flag just records whether the device
(and its driver) can physically support wakeup events. The
:c:func:`device_set_wakeup_capable()` routine affects this flag.
"
...
"
Whether or not a device is capable of issuing wakeup events is a
hardware matter, and the kernel is responsible for keeping track of it.
"
But how is the virtio-balloon device capable of waking up the machine?
Your patch merely implies that the virtio-baloon device is capable to
prohbit going to sleep.
What am I missing?
The underlying virtio_pci_device is capable of waking up the machine,
if it supports PCI power management. The core PCI code will keep the
machine awake while processing the interrupt (i.e. during
virtballoon_changed), but after processing is handed off to the
virtio-balloon driver, the virtio-balloon driver needs to keep the
machine awake until the processing is actually completed.
An alternative to making vb->vdev->dev wakeup capable is to plumb the
pm_stay_awake/pm_relax calls to the underlying virtio_pci_device. Would
that be a preferable approach?
The way you describe it, it rather belongs into the PCI code, because
that's what actually makes the device PM-capable (i.e., would not apply
to virtio-ccw or virtio-mmio). The virtio-balloon itself is not
PM-capable. But how hard is it to move that handling into PCI specific code?
--
Cheers,
David / dhildenb
