Re: [RFC PATCH] virtio_balloon: add param to skip adjusting pages
On Thu, Nov 18, 2021 at 10:25:45AM +0900, David Stevens wrote: > On Wed, Nov 17, 2021 at 10:32 PM Michael S. Tsirkin wrote: > > > > On Wed, Nov 17, 2021 at 07:06:34PM +0900, David Stevens wrote: > > > From: David Stevens > > > > > > Add a module parameters to virtio_balloon to allow specifying whether or > > > not the driver should call adjust_managed_page_count. If the parameter > > > is set, it overrides the default behavior inferred from the deflate on > > > OOM flag. This allows the balloon to operate without changing the amount > > > of memory visible to userspace via /proc/meminfo or sysinfo, even on a > > > system that cannot set the default on OOM flag. > > > > > > The motivation for this patch is to allow userspace to more accurately > > > take advantage of virtio_balloon's cooperative memory control on a > > > system without the ability to use deflate on OOM. As it stands, > > > userspace has no way to know how much memory may be available on such a > > > system, which makes tasks such as sizing caches impossible. > > > > > > When deflate on OOM is not enabled, the current behavior of the > > > virtio_balloon more or less resembles hotplugging individual pages, at > > > least from an accounting perspective. This is basically hardcoding the > > > requirement that totalram_pages must be available to the guest > > > immediately, regardless of what the host does. While that is a valid > > > policy, on Linux (which supports memory overcommit) with virtio_balloon > > > (which is designed to facilitate overcommit in the host), it is not the > > > only possible policy. > > > > > > The param added by this patch allows the guest to operate under the > > > assumption that pages in the virtio_balloon will generally be made > > > available when needed. This assumption may not always hold, but when it > > > is violated, the guest will just fall back to the normal mechanisms for > > > dealing with overcommitted memory. > > > > > > Signed-off-by: David Stevens > > > --- > > > > > > Another option to achieve similar behavior would be to add a new feature > > > flag that would be used in conjunction with DEFLATE_ON_OOM to determine > > > whether or not adjust_managed_page_count should be called. However, I > > > feel that this sort of policy decision is a little outside the scope of > > > what the virtio_balloon API deals with. > > > > > > --- > > > drivers/virtio/virtio_balloon.c | 23 ++- > > > 1 file changed, 18 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/virtio/virtio_balloon.c > > > b/drivers/virtio/virtio_balloon.c > > > index c22ff0117b46..17dac286899c 100644 > > > --- a/drivers/virtio/virtio_balloon.c > > > +++ b/drivers/virtio/virtio_balloon.c > > > @@ -133,6 +133,21 @@ static const struct virtio_device_id id_table[] = { > > > { 0 }, > > > }; > > > > > > +static char *adjust_pages = ""; > > > +module_param(adjust_pages, charp, 0); > > > +MODULE_PARM_DESC(adjust_pages, "Whether or not pages in the balloon > > > should be removed from the managed page count"); > > > + > > > +static bool should_adjust_pages(struct virtio_balloon *vb) > > > +{ > > > + if (!strcmp(adjust_pages, "always")) > > > + return true; > > > + else if (!strcmp(adjust_pages, "never")) > > > + return false; > > > + > > > + return !virtio_has_feature(vb->vdev, > > > +VIRTIO_BALLOON_F_DEFLATE_ON_OOM); > > > +} > > > + > > > static u32 page_to_balloon_pfn(struct page *page) > > > { > > > unsigned long pfn = page_to_pfn(page); > > > @@ -243,8 +258,7 @@ static unsigned fill_balloon(struct virtio_balloon > > > *vb, size_t num) > > > > > > set_page_pfns(vb, vb->pfns + vb->num_pfns, page); > > > vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE; > > > - if (!virtio_has_feature(vb->vdev, > > > - VIRTIO_BALLOON_F_DEFLATE_ON_OOM)) > > > + if (should_adjust_pages(vb)) > > > adjust_managed_page_count(page, -1); > > > vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE; > > > } > > > @@ -264,8 +278,7 @@ static void release_pages_balloon(struct > > > virtio_balloon *vb, > > > struct page *page, *next; > > > > > > list_for_each_entry_safe(page, next, pages, lru) { > > > - if (!virtio_has_feature(vb->vdev, > > > - VIRTIO_BALLOON_F_DEFLATE_ON_OOM)) > > > + if (should_adjust_pages(vb)) > > > adjust_managed_page_count(page, 1); > > > list_del(&page->lru); > > > put_page(page); /* balloon reference */ > > > @@ -775,7 +788,7 @@ static int virtballoon_migratepage(struct > > > balloon_dev_info *vb_dev_info, > > > * managed page count when inflating, we have to fixup the count of > > > * both involved zones. > > > */ > > > - if (!virtio_has_feature(vb->vdev, VIRTIO_BAL
Re: [PATCH 01/29] nvdimm/pmem: move dax_attribute_group from dax to pmem
On Tue, Nov 9, 2021 at 12:33 AM Christoph Hellwig wrote: > > dax_attribute_group is only used by the pmem driver, and can avoid the > completely pointless lookup by the disk name if moved there. This > leaves just a single caller of dax_get_by_host, so move dax_get_by_host > into the same ifdef block as that caller. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Dan Williams > Link: https://lore.kernel.org/r/20210922173431.2454024-3-...@lst.de > Signed-off-by: Dan Williams This one already made v5.16-rc1. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 03/29] dax: remove CONFIG_DAX_DRIVER
On Tue, Nov 9, 2021 at 12:33 AM Christoph Hellwig wrote: > > CONFIG_DAX_DRIVER only selects CONFIG_DAX now, so remove it. Applied. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 02/29] dm: make the DAX support dependend on CONFIG_FS_DAX
On Tue, Nov 9, 2021 at 12:33 AM Christoph Hellwig wrote: > > The device mapper DAX support is all hanging off a block device and thus > can't be used with device dax. Make it depend on CONFIG_FS_DAX instead > of CONFIG_DAX_DRIVER. This also means that bdev_dax_pgoff only needs to > be built under CONFIG_FS_DAX now. > Applied, fixed the spelling of 'dependent' in the subject and picked up Mike's Ack from the previous send: https://lore.kernel.org/r/yyasbvuorceds...@redhat.com Christoph, any particular reason you did not pick up the tags from the last posting? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH] virtio_balloon: add param to skip adjusting pages
On Wed, Nov 17, 2021 at 07:06:34PM +0900, David Stevens wrote: > From: David Stevens > > Add a module parameters to virtio_balloon to allow specifying whether or > not the driver should call adjust_managed_page_count. If the parameter > is set, it overrides the default behavior inferred from the deflate on > OOM flag. This allows the balloon to operate without changing the amount > of memory visible to userspace via /proc/meminfo or sysinfo, even on a > system that cannot set the default on OOM flag. > > The motivation for this patch is to allow userspace to more accurately > take advantage of virtio_balloon's cooperative memory control on a > system without the ability to use deflate on OOM. As it stands, > userspace has no way to know how much memory may be available on such a > system, which makes tasks such as sizing caches impossible. > > When deflate on OOM is not enabled, the current behavior of the > virtio_balloon more or less resembles hotplugging individual pages, at > least from an accounting perspective. This is basically hardcoding the > requirement that totalram_pages must be available to the guest > immediately, regardless of what the host does. While that is a valid > policy, on Linux (which supports memory overcommit) with virtio_balloon > (which is designed to facilitate overcommit in the host), it is not the > only possible policy. > > The param added by this patch allows the guest to operate under the > assumption that pages in the virtio_balloon will generally be made > available when needed. This assumption may not always hold, but when it > is violated, the guest will just fall back to the normal mechanisms for > dealing with overcommitted memory. > > Signed-off-by: David Stevens > --- > > Another option to achieve similar behavior would be to add a new feature > flag that would be used in conjunction with DEFLATE_ON_OOM to determine > whether or not adjust_managed_page_count should be called. However, I > feel that this sort of policy decision is a little outside the scope of > what the virtio_balloon API deals with. > > --- > drivers/virtio/virtio_balloon.c | 23 ++- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index c22ff0117b46..17dac286899c 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -133,6 +133,21 @@ static const struct virtio_device_id id_table[] = { > { 0 }, > }; > > +static char *adjust_pages = ""; > +module_param(adjust_pages, charp, 0); > +MODULE_PARM_DESC(adjust_pages, "Whether or not pages in the balloon should > be removed from the managed page count"); > + > +static bool should_adjust_pages(struct virtio_balloon *vb) > +{ > + if (!strcmp(adjust_pages, "always")) > + return true; > + else if (!strcmp(adjust_pages, "never")) > + return false; > + > + return !virtio_has_feature(vb->vdev, > +VIRTIO_BALLOON_F_DEFLATE_ON_OOM); > +} > + > static u32 page_to_balloon_pfn(struct page *page) > { > unsigned long pfn = page_to_pfn(page); > @@ -243,8 +258,7 @@ static unsigned fill_balloon(struct virtio_balloon *vb, > size_t num) > > set_page_pfns(vb, vb->pfns + vb->num_pfns, page); > vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE; > - if (!virtio_has_feature(vb->vdev, > - VIRTIO_BALLOON_F_DEFLATE_ON_OOM)) > + if (should_adjust_pages(vb)) > adjust_managed_page_count(page, -1); > vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE; > } > @@ -264,8 +278,7 @@ static void release_pages_balloon(struct virtio_balloon > *vb, > struct page *page, *next; > > list_for_each_entry_safe(page, next, pages, lru) { > - if (!virtio_has_feature(vb->vdev, > - VIRTIO_BALLOON_F_DEFLATE_ON_OOM)) > + if (should_adjust_pages(vb)) > adjust_managed_page_count(page, 1); > list_del(&page->lru); > put_page(page); /* balloon reference */ > @@ -775,7 +788,7 @@ static int virtballoon_migratepage(struct > balloon_dev_info *vb_dev_info, > * managed page count when inflating, we have to fixup the count of > * both involved zones. > */ > - if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM) && > + if (should_adjust_pages(vb) && > page_zone(page) != page_zone(newpage)) { > adjust_managed_page_count(page, 1); > adjust_managed_page_count(newpage, -1); A problem here is that the host also cares: IIUC with VIRTIO_BALLOON_F_STATS_VQ we are sending the info about page counts to host, changing what the stats mean. So I suspect we need a device feature for this at least to control this aspect. > -- > 2.34.0.rc2.393.gf8c9666880-goog _
Re: [PATCH] virtio-blk: modify the value type of num in virtio_queue_rq()
On Wed, Nov 17, 2021 at 06:39:55AM +, cgel@gmail.com wrote: From: Ye Guojin This was found by coccicheck: ./drivers/block/virtio_blk.c, 334, 14-17, WARNING Unsigned expression compared with zero num < 0 We should add the Fixes tag: Fixes: 02746e26c39e ("virtio-blk: avoid preallocating big SGL for data") Reported-by: Zeal Robot Signed-off-by: Ye Guojin --- drivers/block/virtio_blk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 97bf051a50ce..eed1666eff31 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -316,7 +316,7 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req = bd->rq; struct virtblk_req *vbr = blk_mq_rq_to_pdu(req); unsigned long flags; - unsigned int num; + int num; int qid = hctx->queue_num; bool notify = false; blk_status_t status; -- 2.25.1 The patch LGTM. With the Fixes tag added: Reviewed-by: Stefano Garzarella ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization