On 04/09/2018 02:03 PM, Michael S. Tsirkin wrote:
On Fri, Apr 06, 2018 at 08:17:23PM +0800, Wei Wang wrote:
Negotiation of the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature indicates the
support of reporting hints of guest free pages to host via virtio-balloon.
Host requests the guest to report free page hints 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 polls the free
page vq after sending the starting cmd id, so the guest doesn't need to
kick after filling an element to the vq.
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@xxxxxxxxx>
Signed-off-by: Liang Li <liang.z.li@xxxxxxxxx>
Cc: Michael S. Tsirkin <mst@xxxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxxxx>
Pretty good by now, Minor comments below.
Thanks for the comments.
---
drivers/virtio/virtio_balloon.c | 272 +++++++++++++++++++++++++++++++-----
include/uapi/linux/virtio_balloon.h | 4 +
2 files changed, 240 insertions(+), 36 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index dfe5684..aef73ee 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -51,9 +51,22 @@ MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
static struct vfsmount *balloon_mnt;
#endif
+enum virtio_balloon_vq {
+ VIRTIO_BALLOON_VQ_INFLATE,
+ VIRTIO_BALLOON_VQ_DEFLATE,
+ VIRTIO_BALLOON_VQ_STATS,
+ VIRTIO_BALLOON_VQ_FREE_PAGE,
+ VIRTIO_BALLOON_VQ_MAX
+};
+
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 +76,13 @@ struct virtio_balloon {
spinlock_t stop_update_lock;
bool stop_update;
+ /* The new cmd id received from host */
+ uint32_t cmd_id_received;
+ /* The cmd id that is in use */
+ __virtio32 cmd_id_use;
I'd prefer cmd_id_active but it's not critical.
OK, will change.
+
+static void report_free_page_func(struct work_struct *work)
+{
+ struct virtio_balloon *vb;
+ struct virtqueue *vq;
+ unsigned int unused;
+ int ret;
+
+ vb = container_of(work, struct virtio_balloon, report_free_page_work);
+ vq = vb->free_page_vq;
+
+ /* Start by sending the received cmd id to host with an outbuf. */
+ ret = send_start_cmd_id(vb, vb->cmd_id_received);
+ if (unlikely(ret))
+ goto err;
+
+ ret = walk_free_mem_block(vb, 0, &virtio_balloon_send_free_pages);
+ if (unlikely(ret == -EIO))
+ goto err;
why is EIO special? I think you should special-case EINTR maybe.
Actually EINTR isn't an error which needs to bail out. That's just the
case that the vq is full, that hint isn't added. Maybe it is not
necessary to treat the "vq full" case as an error.
How about just returning "0" when the vq is full, instead of returning
"EINTR"? (The next hint will continue to be added)
+
+ /* End by sending a stop id to host with an outbuf. */
+ ret = send_stop_cmd_id(vb);
+ if (likely(!ret)) {
What happens on failure? Don't we need to detach anyway?
Yes. Please see below, we could make some more change.
+ /* Ending: detach all the used buffers from the vq. */
+ while (vq->num_free != virtqueue_get_vring_size(vq))
+ virtqueue_get_buf(vq, &unused);
This isn't all that happens here. It also waits for buffers to
be consumed. Is this by design? And why is it good idea to
busy poll while doing it?
Because host and guest operations happen asynchronously. When the guest
reaches here, host may have not put anything to the vq yet. How about
doing this via the free page vq handler?
Host will send a free page vq interrupt before exiting the optimization.
I think this would be nicer.
Best,
Wei
