search for: return_free_pages_to_mm

Since virtio-ccw doesn't work with accessing to the config space inside an interrupt context, this patch series avoids that issue by moving the config register accesses to the related workqueue contexts. v1->v2 ChangeLog: - add config_read_bitmap to indicate to the workqueue callbacks about the necessity of reading the related config fields. Wei Wang (2): virtio-balloon: tweak

...op_update_lock; bool stop_update; + /* Bitmap to indicate if reading the related config fields are needed */ + unsigned long config_read_bitmap; /* The list of allocated free pages, waiting to be given back to mm */ struct list_head free_page_list; @@ -390,37 +396,31 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb, return num_returned; } +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) +{ + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) + return; + + /* No need to queue the work if the bit was already set. */ + if (test_and_se...

Since virtio-ccw doesn't work with accessing to the config registers inside an interrupt context, this patch series avoids that issue by moving the config register accesses to the related workqueue contexts. Wei Wang (2): virtio-balloon: tweak config_changed implementation virtio-balloon: improve update_balloon_size_func drivers/virtio/virtio_balloon.c | 59

...irtio_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_CMD_ID_DONE) { - /* Pass ULONG_MAX to give back all the free pages */ - return_free_pages_to_mm(vb, ULONG_MAX); - } else if (vb->cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && - vb->cmd_id_received != - virtio32_to_cpu(vdev, vb->cmd_id_active)) { - spin_lock_irqsave(&vb->stop_update_lock, flags); - if (!vb->stop_update) { - queue_work(vb->ball...

Since virtio-ccw doesn't work with accessing to the config space inside an interrupt context, this patch series avoids that issue by moving the config register accesses to the related workqueue contexts. v2->v3 ChangeLog: - rename cmd_id_received to cmd_id_received_cache, and have call sites read the latest value via virtio_balloon_cmd_id_received. (Still kept Cornelia and

...:virtio_balloon_get_config() -> if (dev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) -> config.free_page_report_cmd_id = ... VIRTIO_BALLOON_CMD_ID_DONE Linux: drivers/virtio/virtio_balloon.c:report_free_page_func() -> if (cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { -> return_free_pages_to_mm() So it's VIRTIO_BALLOON_CMD_ID_DONE. -- Thanks, David / dhildenb

...> /* The list of allocated free pages, waiting to be given back to mm */ > struct list_head free_page_list; It seems that you never initialize this bitmap. Probably harmless here but generally using uninitialized memory isn't good. > @@ -390,37 +396,31 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb, > return num_returned; > } > > +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) > +{ > + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > + return; > + > + /* No need to queue the work if...

...> /* The list of allocated free pages, waiting to be given back to mm */ > struct list_head free_page_list; It seems that you never initialize this bitmap. Probably harmless here but generally using uninitialized memory isn't good. > @@ -390,37 +396,31 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb, > return num_returned; > } > > +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) > +{ > + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > + return; > + > + /* No need to queue the work if...

...set some kind of flag whenever there is a change interrupt, then have send_free_pages test it and re-read cmd_id_received. Needs to be an atomic I guess ... > - if (vb->cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > - /* Pass ULONG_MAX to give back all the free pages */ > - return_free_pages_to_mm(vb, ULONG_MAX); > - } else if (vb->cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > - vb->cmd_id_received != > - virtio32_to_cpu(vdev, vb->cmd_id_active)) { > - spin_lock_irqsave(&vb->stop_update_lock, flags); > - if (!vb->stop_update) { &g...

...emove_common(struct virtio_balloon *vb) leak_balloon(vb, vb->num_pages); update_balloon_size(vb); + /* There might be free pages that are being reported: release them. */ + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) + return_free_pages_to_mm(vb, ULONG_MAX); + /* Now we reset the device so we can clean up the queues. */ vb->vdev->config->reset(vb->vdev); Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be that we do have VIRTIO_BALLOON_F_MUST_TELL_HOST. I assume this means (-ENOCL...

...emove_common(struct virtio_balloon *vb) leak_balloon(vb, vb->num_pages); update_balloon_size(vb); + /* There might be free pages that are being reported: release them. */ + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) + return_free_pages_to_mm(vb, ULONG_MAX); + /* Now we reset the device so we can clean up the queues. */ vb->vdev->config->reset(vb->vdev); Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be that we do have VIRTIO_BALLOON_F_MUST_TELL_HOST. I assume this means (-ENOCL...

...> leak_balloon(vb, vb->num_pages); > update_balloon_size(vb); > > + /* There might be free pages that are being reported: release them. > */ > + if (virtio_has_feature(vb->vdev, > VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > + return_free_pages_to_mm(vb, ULONG_MAX); > + > /* Now we reset the device so we can clean up the queues. */ > vb->vdev->config->reset(vb->vdev); Right, thanks! > > > Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be that > we do have VIRTIO_B...

...balloon.c @@ -968,6 +968,10 @@ static void remove_common(struct virtio_balloon *vb) leak_balloon(vb, vb->num_pages); update_balloon_size(vb); + /* There might be free pages that are being reported: release them. */ + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) + return_free_pages_to_mm(vb, ULONG_MAX); + /* Now we reset the device so we can clean up the queues. */ vb->vdev->config->reset(vb->vdev); -- 2.24.1

...ev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) > -> config.free_page_report_cmd_id = ... VIRTIO_BALLOON_CMD_ID_DONE > > > Linux: > > drivers/virtio/virtio_balloon.c:report_free_page_func() > -> if (cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > -> return_free_pages_to_mm() > > > So it's VIRTIO_BALLOON_CMD_ID_DONE. > > > -- > Thanks, > > David / dhildenb

...loon *vb) > leak_balloon(vb, vb->num_pages); > update_balloon_size(vb); > > + /* There might be free pages that are being reported: release them. */ > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > + return_free_pages_to_mm(vb, ULONG_MAX); > + > /* Now we reset the device so we can clean up the queues. */ > vb->vdev->config->reset(vb->vdev); Indeed. > > Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be > that we do have VIRTIO_BALLOON_F_MUST...

>>> I would say reverting probably makes sense. I'm not sure there is much >>> value to having a shrinker running deflation when you are actively trying >>> to increase the balloon. It would make more sense to wait until you are >>> actually about to start hitting oom. >> >> I think the shrinker makes sense for free page hinting feature >>

>>> I would say reverting probably makes sense. I'm not sure there is much >>> value to having a shrinker running deflation when you are actively trying >>> to increase the balloon. It would make more sense to wait until you are >>> actually about to start hitting oom. >> >> I think the shrinker makes sense for free page hinting feature >>

This patch series is separated from the previous "Virtio-balloon Enhancement" series. The new feature, VIRTIO_BALLOON_F_FREE_PAGE_HINT, implemented by this series enables the virtio-balloon driver to report hints of guest free pages to the host. It can be used to accelerate live migration of VMs. Here is an introduction of this usage: Live migration needs to transfer the VM's

This patch series is separated from the previous "Virtio-balloon Enhancement" series. The new feature, VIRTIO_BALLOON_F_FREE_PAGE_HINT, implemented by this series enables the virtio-balloon driver to report hints of guest free pages to the host. It can be used to accelerate live migration of VMs. Here is an introduction of this usage: Live migration needs to transfer the VM's

> What do you call "hinting ends" though? The fact we put > a page in the VQ is not a guarantee that it's been consumed > by the hypervisor. > I'd say hinting ends once the hypervisor sets FREE_PAGE_REPORT_S_DONE. > > I think a strict definition is this: > - hint includes a command ID > - hint implies "page was unused at some point after guest