On Mon, Jun 25, 2018 at 08:05:10PM +0800, Wei Wang wrote:
@@ -326,17 +353,6 @@ static void stats_handle_request(struct virtio_balloon *vb)If there's any buffer after this one we might never get another
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;
@@ -353,6 +369,35 @@ 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;
+ 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);
+ }
+
+ if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+ virtio_cread(vdev, struct virtio_balloon_config,
+ free_page_report_cmd_id, &vb->cmd_id_received);
+ if (vb->cmd_id_received !=
+ VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID &&
+ vb->cmd_id_received != vb->cmd_id_active) {
+ spin_lock_irqsave(&vb->stop_update_lock, flags);
+ if (!vb->stop_update)
+ queue_work(vb->balloon_wq,
+ &vb->report_free_page_work);
+ spin_unlock_irqrestore(&vb->stop_update_lock, flags);
+ }
+ }
+}
+
static void update_balloon_size(struct virtio_balloon *vb)
{
u32 actual = vb->num_pages;
@@ -425,44 +470,253 @@ static void update_balloon_size_func(struct work_struct *work)
queue_work(system_freezable_wq, work);
}
+static void free_page_vq_cb(struct virtqueue *vq)
+{
+ unsigned int len;
+ void *buf;
+ struct virtio_balloon *vb = vq->vdev->priv;
+
+ while (1) {
+ buf = virtqueue_get_buf(vq, &len);
+
+ if (!buf || buf == &vb->cmd_start || buf == &vb->cmd_stop)
+ break;
callback.
+ free_pages((unsigned long)buf, ARRAY_ALLOC_ORDER);Why the change? Is it more likely to happen now?
+ }
+}
+
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[VIRTIO_BALLOON_VQ_MAX];
+ vq_callback_t *callbacks[VIRTIO_BALLOON_VQ_MAX];
+ const char *names[VIRTIO_BALLOON_VQ_MAX];
+ struct scatterlist sg;
+ int ret;
/*
- * We expect two virtqueues: inflate and deflate, and
- * optionally stat.
+ * Inflateq and deflateq are used unconditionally. The names[]
+ * will be NULL if the related feature is not enabled, which will
+ * cause no allocation for the corresponding virtqueue in find_vqs.
*/
- 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 (err)
- return err;
+ callbacks[VIRTIO_BALLOON_VQ_INFLATE] = balloon_ack;
+ names[VIRTIO_BALLOON_VQ_INFLATE] = "inflate";
+ callbacks[VIRTIO_BALLOON_VQ_DEFLATE] = balloon_ack;
+ names[VIRTIO_BALLOON_VQ_DEFLATE] = "deflate";
+ names[VIRTIO_BALLOON_VQ_STATS] = NULL;
+ names[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL;
- vb->inflate_vq = vqs[0];
- vb->deflate_vq = vqs[1];
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
- struct scatterlist sg;
- unsigned int num_stats;
- vb->stats_vq = vqs[2];
+ names[VIRTIO_BALLOON_VQ_STATS] = "stats";
+ callbacks[VIRTIO_BALLOON_VQ_STATS] = stats_request;
+ }
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+ names[VIRTIO_BALLOON_VQ_FREE_PAGE] = "free_page_vq";
+ callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = free_page_vq_cb;
+ }
+
+ ret = vb->vdev->config->find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX,
+ vqs, callbacks, names, NULL, NULL);
+ if (ret)
+ return ret;
+
+ vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
+ vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
+ vb->stats_vq = vqs[VIRTIO_BALLOON_VQ_STATS];
/*
* 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);
- if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL)
- < 0)
- BUG();
+ 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;
+ }
+/*
+ * virtio_balloon_send_hints - send arrays of hints to host
+ * @vb: the virtio_balloon struct
+ * @arrays: the arrays of hints
+ * @array_num: the number of arrays give by the caller
+ * @last_array_hints: the number of hints in the last array
+ *
+ * Send hints to host array by array. This begins by sending a start cmd,
+ * which contains a cmd id received from host and the free page block size in
+ * bytes of each hint. At the end, a stop cmd is sent to host to indicate the
+ * end of this reporting. If host actively requests to stop the reporting, free
+ * the arrays that have not been sent.
+ */
+static void virtio_balloon_send_hints(struct virtio_balloon *vb,
+ __le64 **arrays,
+ uint32_t array_num,
+ uint32_t last_array_hints)
+{
+ int err, i = 0;
+ struct scatterlist sg;
+ struct virtqueue *vq = vb->free_page_vq;
+
+ /* Start by sending the received cmd id to host with an outbuf. */
+ err = send_start_cmd_id(vb);
+ if (unlikely(err))
+ goto out_err;
+ /* Kick host to start taking entries from the vq. */
+ virtqueue_kick(vq);
+
+ for (i = 0; i < array_num; i++) {
+ /*
+ * If a stop id or a new cmd id was just received from host,
+ * stop the reporting, and free the remaining arrays that
+ * haven't been sent to host.
+ */
+ if (vb->cmd_id_received != vb->cmd_id_active)
+ goto out_free;
+
+ if (i + 1 == array_num)
+ sg_init_one(&sg, (void *)arrays[i],
+ last_array_hints * sizeof(__le64));
+ else
+ sg_init_one(&sg, (void *)arrays[i], ARRAY_ALLOC_SIZE);
+ err = virtqueue_add_inbuf(vq, &sg, 1, (void *)arrays[i],
+ GFP_KERNEL);
+ if (unlikely(err))
+ goto out_err;
+ }
+
+ /* End by sending a stop id to host with an outbuf. */
+ err = send_stop_cmd_id(vb);
+ if (unlikely(err))
+ goto out_err;
Don't we need to kick here?
+ int i;Instead of all this mess, how about get_free_pages here as well?
+
+ max_entries = max_free_page_blocks(ARRAY_ALLOC_ORDER);
+ entries_per_page = PAGE_SIZE / sizeof(__le64);
+ entries_per_array = entries_per_page * (1 << ARRAY_ALLOC_ORDER);
+ max_array_num = max_entries / entries_per_array +
+ !!(max_entries % entries_per_array);
+ arrays = kmalloc_array(max_array_num, sizeof(__le64 *), GFP_KERNEL);
Also why do we need GFP_KERNEL for this?
+ if (!arrays)So we are getting a ton of memory here just to free it up a bit later.
+ return NULL;
+
+ for (i = 0; i < max_array_num; i++) {
Why doesn't get_from_free_page_list get the pages from free list for us?
We could also avoid the 1st allocation then - just build a list
of these.
+ arrays[i] =Coding style says:
+ (__le64 *)__get_free_pages(__GFP_ATOMIC | __GFP_NOMEMALLOC,
+ ARRAY_ALLOC_ORDER);
Descendants are always substantially shorter than the parent and
are placed substantially to the right.
+ if (!arrays[i]) {Also if it does fail (small guest), shall we try with less arrays?