| Message ID | 20190812131235.27244-3-nitesh@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: Support for page reporting | expand |
On Mon, 12 Aug 2019 09:12:35 -0400 Nitesh Narayan Lal <nitesh@redhat.com> wrote: > Enables the kernel to negotiate VIRTIO_BALLOON_F_REPORTING feature with > the host. If it is available and page_reporting_flag is set to true, > page_reporting is enabled and its callback is configured along with > the max_pages count which indicates the maximum number of pages that > can be isolated and reported at a time. Currently, only free pages of > order >= (MAX_ORDER - 2) are reported. To prevent any false OOM > max_pages count is set to 16. > > By default page_reporting feature is enabled and gets loaded as soon > as the virtio-balloon driver is loaded. However, it could be disabled > by writing the page_reporting_flag which is a virtio-balloon parameter. > > Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> > --- > drivers/virtio/Kconfig | 1 + > drivers/virtio/virtio_balloon.c | 64 ++++++++++++++++++++++++++++- > include/uapi/linux/virtio_balloon.h | 1 + > 3 files changed, 65 insertions(+), 1 deletion(-) > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 226fbb995fb0..defec00d4ee2 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c (...) > +static void virtballoon_page_reporting_setup(struct virtio_balloon *vb) > +{ > + struct device *dev = &vb->vdev->dev; > + int err; > + > + vb->page_reporting_conf.report = virtballoon_report_pages; > + vb->page_reporting_conf.max_pages = PAGE_REPORTING_MAX_PAGES; > + err = page_reporting_enable(&vb->page_reporting_conf); > + if (err < 0) { > + dev_err(dev, "Failed to enable reporting, err = %d\n", err); > + page_reporting_flag = false; Should we clear the feature bit in this case as well? > + vb->page_reporting_conf.report = NULL; > + vb->page_reporting_conf.max_pages = 0; > + return; > + } > +} > + > static void set_page_pfns(struct virtio_balloon *vb, > __virtio32 pfns[], struct page *page) > { > @@ -476,6 +524,7 @@ static int init_vqs(struct virtio_balloon *vb) > names[VIRTIO_BALLOON_VQ_DEFLATE] = "deflate"; > names[VIRTIO_BALLOON_VQ_STATS] = NULL; > names[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; > + names[VIRTIO_BALLOON_VQ_REPORTING] = NULL; > > if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) { > names[VIRTIO_BALLOON_VQ_STATS] = "stats"; > @@ -487,11 +536,18 @@ static int init_vqs(struct virtio_balloon *vb) > callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; > } > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) { > + names[VIRTIO_BALLOON_VQ_REPORTING] = "reporting_vq"; > + callbacks[VIRTIO_BALLOON_VQ_REPORTING] = balloon_ack; Do we even want to try to set up the reporting queue if reporting has been disabled via module parameter? Might make more sense to not even negotiate the feature bit in that case. > + } > err = vb->vdev->config->find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, > vqs, callbacks, names, NULL, NULL); > if (err) > return err; > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) > + vb->reporting_vq = vqs[VIRTIO_BALLOON_VQ_REPORTING]; > + > 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)) { > @@ -924,6 +980,9 @@ static int virtballoon_probe(struct virtio_device *vdev) > if (err) > goto out_del_balloon_wq; > } > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) && > + page_reporting_flag) > + virtballoon_page_reporting_setup(vb); In that case, you'd only need to check for the feature bit here. > virtio_device_ready(vdev); > > if (towards_target(vb)) > @@ -971,6 +1030,8 @@ static void virtballoon_remove(struct virtio_device *vdev) > destroy_workqueue(vb->balloon_wq); > } > > + if (page_reporting_flag) > + page_reporting_disable(&vb->page_reporting_conf); I think you should not disable it if the feature bit was never offered in the first place, right? > remove_common(vb); > #ifdef CONFIG_BALLOON_COMPACTION > if (vb->vb_dev_info.inode) (...)
On 8/14/19 6:29 AM, Cornelia Huck wrote: > On Mon, 12 Aug 2019 09:12:35 -0400 > Nitesh Narayan Lal <nitesh@redhat.com> wrote: > >> Enables the kernel to negotiate VIRTIO_BALLOON_F_REPORTING feature with >> the host. If it is available and page_reporting_flag is set to true, >> page_reporting is enabled and its callback is configured along with >> the max_pages count which indicates the maximum number of pages that >> can be isolated and reported at a time. Currently, only free pages of >> order >= (MAX_ORDER - 2) are reported. To prevent any false OOM >> max_pages count is set to 16. >> >> By default page_reporting feature is enabled and gets loaded as soon >> as the virtio-balloon driver is loaded. However, it could be disabled >> by writing the page_reporting_flag which is a virtio-balloon parameter. >> >> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> >> --- >> drivers/virtio/Kconfig | 1 + >> drivers/virtio/virtio_balloon.c | 64 ++++++++++++++++++++++++++++- >> include/uapi/linux/virtio_balloon.h | 1 + >> 3 files changed, 65 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c >> index 226fbb995fb0..defec00d4ee2 100644 >> --- a/drivers/virtio/virtio_balloon.c >> +++ b/drivers/virtio/virtio_balloon.c > (...) > >> +static void virtballoon_page_reporting_setup(struct virtio_balloon *vb) >> +{ >> + struct device *dev = &vb->vdev->dev; >> + int err; >> + >> + vb->page_reporting_conf.report = virtballoon_report_pages; >> + vb->page_reporting_conf.max_pages = PAGE_REPORTING_MAX_PAGES; >> + err = page_reporting_enable(&vb->page_reporting_conf); >> + if (err < 0) { >> + dev_err(dev, "Failed to enable reporting, err = %d\n", err); >> + page_reporting_flag = false; > Should we clear the feature bit in this case as well? I think yes. If I am not wrong then in a case where page reporting setup fails for some reason and at a later point the user wants to re-enable it then for that balloon driver has to be reloaded. Which would mean re-negotiation of the feature bit. > >> + vb->page_reporting_conf.report = NULL; >> + vb->page_reporting_conf.max_pages = 0; >> + return; >> + } >> +} >> + >> static void set_page_pfns(struct virtio_balloon *vb, >> __virtio32 pfns[], struct page *page) >> { >> @@ -476,6 +524,7 @@ static int init_vqs(struct virtio_balloon *vb) >> names[VIRTIO_BALLOON_VQ_DEFLATE] = "deflate"; >> names[VIRTIO_BALLOON_VQ_STATS] = NULL; >> names[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; >> + names[VIRTIO_BALLOON_VQ_REPORTING] = NULL; >> >> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) { >> names[VIRTIO_BALLOON_VQ_STATS] = "stats"; >> @@ -487,11 +536,18 @@ static int init_vqs(struct virtio_balloon *vb) >> callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; >> } >> >> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) { >> + names[VIRTIO_BALLOON_VQ_REPORTING] = "reporting_vq"; >> + callbacks[VIRTIO_BALLOON_VQ_REPORTING] = balloon_ack; > Do we even want to try to set up the reporting queue if reporting has > been disabled via module parameter? Might make more sense to not even > negotiate the feature bit in that case. True. I think this should be replaced with something like (page_reporting_flag && virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)). > >> + } >> err = vb->vdev->config->find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, >> vqs, callbacks, names, NULL, NULL); >> if (err) >> return err; >> >> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) >> + vb->reporting_vq = vqs[VIRTIO_BALLOON_VQ_REPORTING]; >> + >> 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)) { >> @@ -924,6 +980,9 @@ static int virtballoon_probe(struct virtio_device *vdev) >> if (err) >> goto out_del_balloon_wq; >> } >> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) && >> + page_reporting_flag) >> + virtballoon_page_reporting_setup(vb); > In that case, you'd only need to check for the feature bit here. Why is that? I think both the checks should be present here as we need both the conditions to be true to enable page reporting. However, the order should be reversed because of the reason you mentioned earlier. > >> virtio_device_ready(vdev); >> >> if (towards_target(vb)) >> @@ -971,6 +1030,8 @@ static void virtballoon_remove(struct virtio_device *vdev) >> destroy_workqueue(vb->balloon_wq); >> } >> >> + if (page_reporting_flag) >> + page_reporting_disable(&vb->page_reporting_conf); > I think you should not disable it if the feature bit was never offered > in the first place, right? Yes. I should check both feature bit and module parameter here. > >> remove_common(vb); >> #ifdef CONFIG_BALLOON_COMPACTION >> if (vb->vb_dev_info.inode) > (...)
On Wed, 14 Aug 2019 07:47:40 -0400 Nitesh Narayan Lal <nitesh@redhat.com> wrote: > On 8/14/19 6:29 AM, Cornelia Huck wrote: > > On Mon, 12 Aug 2019 09:12:35 -0400 > > Nitesh Narayan Lal <nitesh@redhat.com> wrote: > > > >> Enables the kernel to negotiate VIRTIO_BALLOON_F_REPORTING feature with > >> the host. If it is available and page_reporting_flag is set to true, > >> page_reporting is enabled and its callback is configured along with > >> the max_pages count which indicates the maximum number of pages that > >> can be isolated and reported at a time. Currently, only free pages of > >> order >= (MAX_ORDER - 2) are reported. To prevent any false OOM > >> max_pages count is set to 16. > >> > >> By default page_reporting feature is enabled and gets loaded as soon > >> as the virtio-balloon driver is loaded. However, it could be disabled > >> by writing the page_reporting_flag which is a virtio-balloon parameter. > >> > >> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> > >> --- > >> drivers/virtio/Kconfig | 1 + > >> drivers/virtio/virtio_balloon.c | 64 ++++++++++++++++++++++++++++- > >> include/uapi/linux/virtio_balloon.h | 1 + > >> 3 files changed, 65 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > >> index 226fbb995fb0..defec00d4ee2 100644 > >> --- a/drivers/virtio/virtio_balloon.c > >> +++ b/drivers/virtio/virtio_balloon.c > > (...) > > > >> +static void virtballoon_page_reporting_setup(struct virtio_balloon *vb) > >> +{ > >> + struct device *dev = &vb->vdev->dev; > >> + int err; > >> + > >> + vb->page_reporting_conf.report = virtballoon_report_pages; > >> + vb->page_reporting_conf.max_pages = PAGE_REPORTING_MAX_PAGES; > >> + err = page_reporting_enable(&vb->page_reporting_conf); > >> + if (err < 0) { > >> + dev_err(dev, "Failed to enable reporting, err = %d\n", err); > >> + page_reporting_flag = false; > > Should we clear the feature bit in this case as well? > > I think yes. Eww, I didn't recall that we don't call the ->probe callback until after feature negotiation has finished, so scratch that particular idea. For what reasons may page_reporting_enable() fail? Does it make sense to fail probing the device in that case? And does it make sense to re-try later (i.e. leave page_reporting_flag set)? > If I am not wrong then in a case where page reporting setup fails for some > reason and at a later point the user wants to re-enable it then for that balloon > driver has to be reloaded. > Which would mean re-negotiation of the feature bit. Re-negotiation actually already happens if a driver is unbound and rebound. > > > > >> + vb->page_reporting_conf.report = NULL; > >> + vb->page_reporting_conf.max_pages = 0; > >> + return; > >> + } > >> +} > >> + > >> static void set_page_pfns(struct virtio_balloon *vb, > >> __virtio32 pfns[], struct page *page) > >> { > >> @@ -476,6 +524,7 @@ static int init_vqs(struct virtio_balloon *vb) > >> names[VIRTIO_BALLOON_VQ_DEFLATE] = "deflate"; > >> names[VIRTIO_BALLOON_VQ_STATS] = NULL; > >> names[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; > >> + names[VIRTIO_BALLOON_VQ_REPORTING] = NULL; > >> > >> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) { > >> names[VIRTIO_BALLOON_VQ_STATS] = "stats"; > >> @@ -487,11 +536,18 @@ static int init_vqs(struct virtio_balloon *vb) > >> callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; > >> } > >> > >> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) { > >> + names[VIRTIO_BALLOON_VQ_REPORTING] = "reporting_vq"; > >> + callbacks[VIRTIO_BALLOON_VQ_REPORTING] = balloon_ack; > > Do we even want to try to set up the reporting queue if reporting has > > been disabled via module parameter? Might make more sense to not even > > negotiate the feature bit in that case. > > True. > I think this should be replaced with something like (page_reporting_flag && > virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)). Yes. Is page_reporting_flag supposed to be changeable on the fly? The only way to really turn off the feature bit from the driver is to not pass in the feature in the features table; we could provide two different tables depending on the flag if it were static. > > > > >> + } > >> err = vb->vdev->config->find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, > >> vqs, callbacks, names, NULL, NULL); > >> if (err) > >> return err; > >> > >> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) > >> + vb->reporting_vq = vqs[VIRTIO_BALLOON_VQ_REPORTING]; > >> + > >> 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)) { > >> @@ -924,6 +980,9 @@ static int virtballoon_probe(struct virtio_device *vdev) > >> if (err) > >> goto out_del_balloon_wq; > >> } > >> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) && > >> + page_reporting_flag) > >> + virtballoon_page_reporting_setup(vb); > > In that case, you'd only need to check for the feature bit here. > > Why is that? > I think both the checks should be present here as we need both the conditions to > be true to enable page reporting. Yeah, because we can't clear the feature bit if the flag is not set. > However, the order should be reversed because of the reason you mentioned earlier. > > > > >> virtio_device_ready(vdev); > >> > >> if (towards_target(vb))
On 8/14/19 9:42 AM, Cornelia Huck wrote: > On Wed, 14 Aug 2019 07:47:40 -0400 > Nitesh Narayan Lal <nitesh@redhat.com> wrote: > >> On 8/14/19 6:29 AM, Cornelia Huck wrote: >>> On Mon, 12 Aug 2019 09:12:35 -0400 >>> Nitesh Narayan Lal <nitesh@redhat.com> wrote: >>> >>>> Enables the kernel to negotiate VIRTIO_BALLOON_F_REPORTING feature with >>>> the host. If it is available and page_reporting_flag is set to true, >>>> page_reporting is enabled and its callback is configured along with >>>> the max_pages count which indicates the maximum number of pages that >>>> can be isolated and reported at a time. Currently, only free pages of >>>> order >= (MAX_ORDER - 2) are reported. To prevent any false OOM >>>> max_pages count is set to 16. >>>> >>>> By default page_reporting feature is enabled and gets loaded as soon >>>> as the virtio-balloon driver is loaded. However, it could be disabled >>>> by writing the page_reporting_flag which is a virtio-balloon parameter. >>>> >>>> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> >>>> --- >>>> drivers/virtio/Kconfig | 1 + >>>> drivers/virtio/virtio_balloon.c | 64 ++++++++++++++++++++++++++++- >>>> include/uapi/linux/virtio_balloon.h | 1 + >>>> 3 files changed, 65 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c >>>> index 226fbb995fb0..defec00d4ee2 100644 >>>> --- a/drivers/virtio/virtio_balloon.c >>>> +++ b/drivers/virtio/virtio_balloon.c >>> (...) >>> >>>> +static void virtballoon_page_reporting_setup(struct virtio_balloon *vb) >>>> +{ >>>> + struct device *dev = &vb->vdev->dev; >>>> + int err; >>>> + >>>> + vb->page_reporting_conf.report = virtballoon_report_pages; >>>> + vb->page_reporting_conf.max_pages = PAGE_REPORTING_MAX_PAGES; >>>> + err = page_reporting_enable(&vb->page_reporting_conf); >>>> + if (err < 0) { >>>> + dev_err(dev, "Failed to enable reporting, err = %d\n", err); >>>> + page_reporting_flag = false; >>> Should we clear the feature bit in this case as well? >> I think yes. > Eww, I didn't recall that we don't call the ->probe callback until > after feature negotiation has finished, so scratch that particular idea. > > For what reasons may page_reporting_enable() fail? If the guest is low in memory and some allocation required for page reporting setup fails. > Does it make sense > to fail probing the device in that case? And does it make sense to > re-try later (i.e. leave page_reporting_flag set)? Re-trying to setup page reporting will mean that virtballoon_probe has to be called again. For which the driver has to be re-loaded, isn't? > >> If I am not wrong then in a case where page reporting setup fails for some >> reason and at a later point the user wants to re-enable it then for that balloon >> driver has to be reloaded. >> Which would mean re-negotiation of the feature bit. > Re-negotiation actually already happens if a driver is unbound and > rebound. > >>> >>>> + vb->page_reporting_conf.report = NULL; >>>> + vb->page_reporting_conf.max_pages = 0; >>>> + return; >>>> + } >>>> +} >>>> + >>>> static void set_page_pfns(struct virtio_balloon *vb, >>>> __virtio32 pfns[], struct page *page) >>>> { >>>> @@ -476,6 +524,7 @@ static int init_vqs(struct virtio_balloon *vb) >>>> names[VIRTIO_BALLOON_VQ_DEFLATE] = "deflate"; >>>> names[VIRTIO_BALLOON_VQ_STATS] = NULL; >>>> names[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; >>>> + names[VIRTIO_BALLOON_VQ_REPORTING] = NULL; >>>> >>>> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) { >>>> names[VIRTIO_BALLOON_VQ_STATS] = "stats"; >>>> @@ -487,11 +536,18 @@ static int init_vqs(struct virtio_balloon *vb) >>>> callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; >>>> } >>>> >>>> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) { >>>> + names[VIRTIO_BALLOON_VQ_REPORTING] = "reporting_vq"; >>>> + callbacks[VIRTIO_BALLOON_VQ_REPORTING] = balloon_ack; >>> Do we even want to try to set up the reporting queue if reporting has >>> been disabled via module parameter? Might make more sense to not even >>> negotiate the feature bit in that case. >> True. >> I think this should be replaced with something like (page_reporting_flag && >> virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)). > Yes. > > Is page_reporting_flag supposed to be changeable on the fly? Yes. > The only > way to really turn off the feature bit from the driver is to not pass > in the feature in the features table; we could provide two different > tables depending on the flag if it were static. I did have a plan of moving to static keys eventually instead of using module parameters for this purpose. :) That way I will be able to just control the kernel side of things on the fly without changing the balloon-page-reporting framework. The objective is to allow the user to enable/disable page tracking on the fly. > >>> >>>> + } >>>> err = vb->vdev->config->find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, >>>> vqs, callbacks, names, NULL, NULL); >>>> if (err) >>>> return err; >>>> >>>> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) >>>> + vb->reporting_vq = vqs[VIRTIO_BALLOON_VQ_REPORTING]; >>>> + >>>> 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)) { >>>> @@ -924,6 +980,9 @@ static int virtballoon_probe(struct virtio_device *vdev) >>>> if (err) >>>> goto out_del_balloon_wq; >>>> } >>>> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) && >>>> + page_reporting_flag) >>>> + virtballoon_page_reporting_setup(vb); >>> In that case, you'd only need to check for the feature bit here. >> Why is that? >> I think both the checks should be present here as we need both the conditions to >> be true to enable page reporting. > Yeah, because we can't clear the feature bit if the flag is not set. +1 > >> However, the order should be reversed because of the reason you mentioned earlier. >> >>> >>>> virtio_device_ready(vdev); >>>> >>>> if (towards_target(vb))
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 078615cf2afc..4b2dd8259ff5 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -58,6 +58,7 @@ config VIRTIO_BALLOON tristate "Virtio balloon driver" depends on VIRTIO select MEMORY_BALLOON + select PAGE_REPORTING ---help--- This driver supports increasing and decreasing the amount of memory within a KVM guest. diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 226fbb995fb0..defec00d4ee2 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -19,6 +19,7 @@ #include <linux/mount.h> #include <linux/magic.h> #include <linux/pseudo_fs.h> +#include <linux/page_reporting.h> /* * Balloon device works in 4K page units. So each page is pointed to by @@ -46,6 +47,7 @@ enum virtio_balloon_vq { VIRTIO_BALLOON_VQ_DEFLATE, VIRTIO_BALLOON_VQ_STATS, VIRTIO_BALLOON_VQ_FREE_PAGE, + VIRTIO_BALLOON_VQ_REPORTING, VIRTIO_BALLOON_VQ_MAX }; @@ -55,7 +57,8 @@ enum virtio_balloon_config_read { struct virtio_balloon { struct virtio_device *vdev; - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq; + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq, + *reporting_vq; /* Balloon's own wq for cpu-intensive work items */ struct workqueue_struct *balloon_wq; @@ -113,6 +116,9 @@ struct virtio_balloon { /* To register a shrinker to shrink memory upon memory pressure */ struct shrinker shrinker; + + /* To configure page reporting to report isolated pages */ + struct page_reporting_config page_reporting_conf; }; static struct virtio_device_id id_table[] = { @@ -120,6 +126,10 @@ static struct virtio_device_id id_table[] = { { 0 }, }; +bool page_reporting_flag = true; +module_param(page_reporting_flag, bool, 0644); +MODULE_PARM_DESC(page_reporting_flag, "Enable page reporting"); + static u32 page_to_balloon_pfn(struct page *page) { unsigned long pfn = page_to_pfn(page); @@ -152,6 +162,44 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq) } +void virtballoon_report_pages(struct page_reporting_config *page_reporting_conf, + unsigned int num_pages) +{ + struct virtio_balloon *vb = container_of(page_reporting_conf, + struct virtio_balloon, + page_reporting_conf); + struct virtqueue *vq = vb->reporting_vq; + int err, unused; + + /* We should always be able to add these buffers to an empty queue. */ + err = virtqueue_add_inbuf(vq, page_reporting_conf->sg, num_pages, vb, + GFP_NOWAIT); + /* We should not report if the guest is low on memory */ + if (unlikely(err)) + return; + virtqueue_kick(vq); + + /* When host has read buffer, this completes via balloon_ack */ + wait_event(vb->acked, virtqueue_get_buf(vq, &unused)); +} + +static void virtballoon_page_reporting_setup(struct virtio_balloon *vb) +{ + struct device *dev = &vb->vdev->dev; + int err; + + vb->page_reporting_conf.report = virtballoon_report_pages; + vb->page_reporting_conf.max_pages = PAGE_REPORTING_MAX_PAGES; + err = page_reporting_enable(&vb->page_reporting_conf); + if (err < 0) { + dev_err(dev, "Failed to enable reporting, err = %d\n", err); + page_reporting_flag = false; + vb->page_reporting_conf.report = NULL; + vb->page_reporting_conf.max_pages = 0; + return; + } +} + static void set_page_pfns(struct virtio_balloon *vb, __virtio32 pfns[], struct page *page) { @@ -476,6 +524,7 @@ static int init_vqs(struct virtio_balloon *vb) names[VIRTIO_BALLOON_VQ_DEFLATE] = "deflate"; names[VIRTIO_BALLOON_VQ_STATS] = NULL; names[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; + names[VIRTIO_BALLOON_VQ_REPORTING] = NULL; if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) { names[VIRTIO_BALLOON_VQ_STATS] = "stats"; @@ -487,11 +536,18 @@ static int init_vqs(struct virtio_balloon *vb) callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; } + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) { + names[VIRTIO_BALLOON_VQ_REPORTING] = "reporting_vq"; + callbacks[VIRTIO_BALLOON_VQ_REPORTING] = balloon_ack; + } err = vb->vdev->config->find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, vqs, callbacks, names, NULL, NULL); if (err) return err; + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) + vb->reporting_vq = vqs[VIRTIO_BALLOON_VQ_REPORTING]; + 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)) { @@ -924,6 +980,9 @@ static int virtballoon_probe(struct virtio_device *vdev) if (err) goto out_del_balloon_wq; } + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) && + page_reporting_flag) + virtballoon_page_reporting_setup(vb); virtio_device_ready(vdev); if (towards_target(vb)) @@ -971,6 +1030,8 @@ static void virtballoon_remove(struct virtio_device *vdev) destroy_workqueue(vb->balloon_wq); } + if (page_reporting_flag) + page_reporting_disable(&vb->page_reporting_conf); remove_common(vb); #ifdef CONFIG_BALLOON_COMPACTION if (vb->vb_dev_info.inode) @@ -1027,6 +1088,7 @@ static unsigned int features[] = { VIRTIO_BALLOON_F_DEFLATE_ON_OOM, VIRTIO_BALLOON_F_FREE_PAGE_HINT, VIRTIO_BALLOON_F_PAGE_POISON, + VIRTIO_BALLOON_F_REPORTING, }; static struct virtio_driver virtio_balloon_driver = { diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h index a1966cd7b677..19974392d324 100644 --- a/include/uapi/linux/virtio_balloon.h +++ b/include/uapi/linux/virtio_balloon.h @@ -36,6 +36,7 @@ #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */ #define VIRTIO_BALLOON_F_FREE_PAGE_HINT 3 /* VQ to report free pages */ #define VIRTIO_BALLOON_F_PAGE_POISON 4 /* Guest is using page poisoning */ +#define VIRTIO_BALLOON_F_REPORTING 5 /* Page reporting virtqueue */ /* Size of a PFN in the balloon interface. */ #define VIRTIO_BALLOON_PFN_SHIFT 12
Enables the kernel to negotiate VIRTIO_BALLOON_F_REPORTING feature with the host. If it is available and page_reporting_flag is set to true, page_reporting is enabled and its callback is configured along with the max_pages count which indicates the maximum number of pages that can be isolated and reported at a time. Currently, only free pages of order >= (MAX_ORDER - 2) are reported. To prevent any false OOM max_pages count is set to 16. By default page_reporting feature is enabled and gets loaded as soon as the virtio-balloon driver is loaded. However, it could be disabled by writing the page_reporting_flag which is a virtio-balloon parameter. Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> --- drivers/virtio/Kconfig | 1 + drivers/virtio/virtio_balloon.c | 64 ++++++++++++++++++++++++++++- include/uapi/linux/virtio_balloon.h | 1 + 3 files changed, 65 insertions(+), 1 deletion(-)