From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2A57C76196 for ; Tue, 16 Jul 2019 15:37:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 49A9A20693 for ; Tue, 16 Jul 2019 15:37:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="OhZ27q56" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 49A9A20693 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B192F8E000E; Tue, 16 Jul 2019 11:37:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AA3FF8E0006; Tue, 16 Jul 2019 11:37:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 96A058E000E; Tue, 16 Jul 2019 11:37:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by kanga.kvack.org (Postfix) with ESMTP id 6FA288E0006 for ; Tue, 16 Jul 2019 11:37:19 -0400 (EDT) Received: by mail-qt1-f197.google.com with SMTP id q26so18433328qtr.3 for ; Tue, 16 Jul 2019 08:37:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:dkim-signature:mime-version:references :in-reply-to:from:date:message-id:subject:to:cc; bh=C93X661VNMOR9DFx3hY8rNDYh7Sxy5r20/CPUz7fPoI=; b=ggdcvBX5eaD0eN8NScALUxwKHIRUEAnH0Q8DELd3vVvGt1s76P0TWGSV0MNdWy9QP1 KqRsHoFe9SO3pMYt12F56RwJW92DOb+b+FE1m9KByqk6kacgSz3uu1sG531MBOBf6xkv 7v+7uYJUqIqYVy9ddNMOIjYENmkzx0RqBQKDnVyZhsRqPWm4WSVAeha+MRskjm7d5fK1 1nKJN0Bmf6GkPLIyEi77AhSYRNlI6aFNUMl369UZHcYeQQN9kdppBmVqDLGHvVSv0B5d j6a/4G36YgBEalyPSg6DeoB06180yAc721E/y0kyzZipqlOBODhsGv+hyCb6uGAAHalA k5aA== X-Gm-Message-State: APjAAAUawgvwrd+KBtepNe0Ei+SIQhUAqqr7WIsRuZ0+RPwQQ1ecx8Zk rOJE9W+AhCP9dNzF3AnI+kqhn4V8Lq4jAwhBWIK0Mf1fUULY0RNwqLeRlyM+fymtBavkQdhDypV 4hw5aSx1cyoeb95ZD3zcrXKqT4Sa8mxhv9/GdexT3s1sSBPihPDg/eMdvSLkXt5qh6g== X-Received: by 2002:a0c:b755:: with SMTP id q21mr23956933qve.92.1563291439119; Tue, 16 Jul 2019 08:37:19 -0700 (PDT) X-Received: by 2002:a0c:b755:: with SMTP id q21mr23956856qve.92.1563291438090; Tue, 16 Jul 2019 08:37:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1563291438; cv=none; d=google.com; s=arc-20160816; b=RKRfbK3GGWA0kOaNMlHUP8XrHmMHnuieqhZun7iTZVdi/xJER2yeDgUjS5zdZuhD6t a1aKd7pIOjV8msXilvfsDMcwgAhqnpjcstAW6fyD61euVV2meKLmWBLYgBA1eFMEMJfn Y/GMgtQ9fmWM5qjtvyRZkaxDwkMzmOgUv96T3snYsIFQ9mcFrgm4zWNRfCv4pUIASGbz NXWyuOIJkk0r0YkLpb8RhCbZTXiQVuPUVyWMKvEhStHGTLIbeRUjsH7NmrBUu1w9Am3m mAuH+7NkHfSghefJZB1M5k0VkqnpF9CemWbO3TYlPIkOz50FrsKP04/NDPFiz3zY9vR6 wzAw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=C93X661VNMOR9DFx3hY8rNDYh7Sxy5r20/CPUz7fPoI=; b=wO48uEI9gbzl/rEne1FrVHjaAE+noGPUdqQzuMjgSQYCl4JyYOhBuumYHn0HxCtbmU Ex1b4gkXjjR2UvqC7MBmw7hEGRg6Yt80RSUHqP0NnlVQpzPpM7vXXev+5jvLEbhw/ATm dB0rbvrltopGERqdBcstbXcyxVrpa/HbtTWYygZAiV2XPOO8P1SwgcaJaECQA8axE9UK +/3cT2OmxJEC7UNL3CrvO+EUPzcEXJGK92vuFW6BfBsd4+iBDQJSiAtjsk4QWMg9SDyO 4E+5QPY7k+fATeUSW/vc7Pg6TBu7EDm38qaN05cUZAxgbmOYMKF5gdqAnADO9taGlrj0 Zd7A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=OhZ27q56; spf=pass (google.com: domain of alexander.duyck@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id o14sor28436736qtg.39.2019.07.16.08.37.18 for (Google Transport Security); Tue, 16 Jul 2019 08:37:18 -0700 (PDT) Received-SPF: pass (google.com: domain of alexander.duyck@gmail.com designates 209.85.220.65 as permitted sender) client-ip=209.85.220.65; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=OhZ27q56; spf=pass (google.com: domain of alexander.duyck@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=C93X661VNMOR9DFx3hY8rNDYh7Sxy5r20/CPUz7fPoI=; b=OhZ27q5680bO3unAKy/oIbRQ/kHunOzN6T3oKB4ku54b6OG12FYS6p/cAWrB12xeTw Dq9rLUvss3pISdY4FOhlS3R69lfLwwFp0qFxo+/hgye4iji9hsVqP7qav+NhvvlfvVmC k0eVvUMY2h5C/ErIkxEBtkrB5wzGn94RCjDNIXqGeuIaK4xcDGXQ0SuNW9huiZQOpeM9 NlMovhx4HIrrk/jTZLvkNF+a07kGkvuia+AIzzQ3470a3e5PhIoplJX3vUKNPVY4gvTB RuN/DgAGzx5EOH0d58n7ZdEmV6WtnVPz4BI+sYEoZGCzFEcwtrv2+b/n7uh6cqhTfVxg WZ1g== X-Google-Smtp-Source: APXvYqy5077FqnRt9dZKnpu3FoJaG6aAk6xbfNR4grwb2kuiH/6uuo0NehX4b2tBaddVUBwvXctFAZweZPx0M0mSfVE= X-Received: by 2002:ac8:32c8:: with SMTP id a8mr22704040qtb.47.1563291437641; Tue, 16 Jul 2019 08:37:17 -0700 (PDT) MIME-Version: 1.0 References: <20190619222922.1231.27432.stgit@localhost.localdomain> <20190619223338.1231.52537.stgit@localhost.localdomain> <20190716055017-mutt-send-email-mst@kernel.org> In-Reply-To: <20190716055017-mutt-send-email-mst@kernel.org> From: Alexander Duyck Date: Tue, 16 Jul 2019 08:37:06 -0700 Message-ID: Subject: Re: [PATCH v1 6/6] virtio-balloon: Add support for aerating memory via hinting To: "Michael S. Tsirkin" Cc: Nitesh Narayan Lal , kvm list , David Hildenbrand , Dave Hansen , LKML , linux-mm , Andrew Morton , Yang Zhang , pagupta@redhat.com, Rik van Riel , Konrad Rzeszutek Wilk , lcapitulino@redhat.com, wei.w.wang@intel.com, Andrea Arcangeli , Paolo Bonzini , dan.j.williams@intel.com, Alexander Duyck Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jul 16, 2019 at 2:55 AM Michael S. Tsirkin wrote: > > On Wed, Jun 19, 2019 at 03:33:38PM -0700, Alexander Duyck wrote: > > From: Alexander Duyck > > > > Add support for aerating memory using the hinting feature provided by > > virtio-balloon. Hinting differs from the regular balloon functionality in > > that is is much less durable than a standard memory balloon. Instead of > > creating a list of pages that cannot be accessed the pages are only > > inaccessible while they are being indicated to the virtio interface. Once > > the interface has acknowledged them they are placed back into their > > respective free lists and are once again accessible by the guest system. > > > > Signed-off-by: Alexander Duyck > > --- > > drivers/virtio/Kconfig | 1 > > drivers/virtio/virtio_balloon.c | 110 ++++++++++++++++++++++++++++++++++- > > include/uapi/linux/virtio_balloon.h | 1 > > 3 files changed, 108 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > > index 023fc3bc01c6..9cdaccf92c3a 100644 > > --- a/drivers/virtio/Kconfig > > +++ b/drivers/virtio/Kconfig > > @@ -47,6 +47,7 @@ config VIRTIO_BALLOON > > tristate "Virtio balloon driver" > > depends on VIRTIO > > select MEMORY_BALLOON > > + select AERATION > > ---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 44339fc87cc7..91f1e8c9017d 100644 > > --- a/drivers/virtio/virtio_balloon.c > > +++ b/drivers/virtio/virtio_balloon.c > > @@ -18,6 +18,7 @@ > > #include > > #include > > #include > > +#include > > > > /* > > * Balloon device works in 4K page units. So each page is pointed to by > > @@ -26,6 +27,7 @@ > > */ > > #define VIRTIO_BALLOON_PAGES_PER_PAGE (unsigned)(PAGE_SIZE >> VIRTIO_BALLOON_PFN_SHIFT) > > #define VIRTIO_BALLOON_ARRAY_PFNS_MAX 256 > > +#define VIRTIO_BALLOON_ARRAY_HINTS_MAX 32 > > #define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80 > > > > #define VIRTIO_BALLOON_FREE_PAGE_ALLOC_FLAG (__GFP_NORETRY | __GFP_NOWARN | \ > > @@ -45,6 +47,7 @@ enum virtio_balloon_vq { > > VIRTIO_BALLOON_VQ_DEFLATE, > > VIRTIO_BALLOON_VQ_STATS, > > VIRTIO_BALLOON_VQ_FREE_PAGE, > > + VIRTIO_BALLOON_VQ_HINTING, > > VIRTIO_BALLOON_VQ_MAX > > }; > > > > @@ -54,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, > > + *hinting_vq; > > > > /* Balloon's own wq for cpu-intensive work items */ > > struct workqueue_struct *balloon_wq; > > @@ -103,9 +107,21 @@ struct virtio_balloon { > > /* Synchronize access/update to this struct virtio_balloon elements */ > > struct mutex balloon_lock; > > > > - /* The array of pfns we tell the Host about. */ > > - unsigned int num_pfns; > > - __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX]; > > + > > + union { > > + /* The array of pfns we tell the Host about. */ > > + struct { > > + unsigned int num_pfns; > > + __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX]; > > + }; > > + /* The array of physical addresses we are hinting on */ > > + struct { > > + unsigned int num_hints; > > + __virtio64 hints[VIRTIO_BALLOON_ARRAY_HINTS_MAX]; > > + }; > > + }; > > + > > + struct aerator_dev_info a_dev_info; > > > > /* Memory statistics */ > > struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR]; > > @@ -151,6 +167,68 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq) > > > > } > > > > +static u64 page_to_hints_pa_order(struct page *page) > > +{ > > + unsigned char order; > > + dma_addr_t pa; > > + > > + BUILD_BUG_ON((64 - VIRTIO_BALLOON_PFN_SHIFT) >= > > + (1 << VIRTIO_BALLOON_PFN_SHIFT)); > > + > > + /* > > + * Record physical page address combined with page order. > > + * Order will never exceed 64 - VIRTIO_BALLON_PFN_SHIFT > > + * since the size has to fit into a 64b value. So as long > > + * as VIRTIO_BALLOON_SHIFT is greater than this combining > > + * the two values should be safe. > > + */ > > + pa = page_to_phys(page); > > + order = page_private(page) + > > + PAGE_SHIFT - VIRTIO_BALLOON_PFN_SHIFT; > > + > > + return (u64)(pa | order); > > +} > > + > > +void virtballoon_aerator_react(struct aerator_dev_info *a_dev_info) > > +{ > > + struct virtio_balloon *vb = container_of(a_dev_info, > > + struct virtio_balloon, > > + a_dev_info); > > + struct virtqueue *vq = vb->hinting_vq; > > + struct scatterlist sg; > > + unsigned int unused; > > + struct page *page; > > + > > + mutex_lock(&vb->balloon_lock); > > + > > + vb->num_hints = 0; > > + > > + list_for_each_entry(page, &a_dev_info->batch, lru) { > > + vb->hints[vb->num_hints++] = > > + cpu_to_virtio64(vb->vdev, > > + page_to_hints_pa_order(page)); > > + } > > + > > + /* We shouldn't have been called if there is nothing to process */ > > + if (WARN_ON(vb->num_hints == 0)) > > + goto out; > > + > > + sg_init_one(&sg, vb->hints, > > + sizeof(vb->hints[0]) * vb->num_hints); > > + > > + /* > > + * We should always be able to add one buffer to an > > + * empty queue. > > + */ > > + virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL); > > + virtqueue_kick(vq); > > + > > + /* When host has read buffer, this completes via balloon_ack */ > > + wait_event(vb->acked, virtqueue_get_buf(vq, &unused)); > > +out: > > + mutex_unlock(&vb->balloon_lock); > > +} > > + > > static void set_page_pfns(struct virtio_balloon *vb, > > __virtio32 pfns[], struct page *page) > > { > > @@ -475,6 +553,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_HINTING] = NULL; > > > > if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) { > > names[VIRTIO_BALLOON_VQ_STATS] = "stats"; > > @@ -486,11 +565,19 @@ static int init_vqs(struct virtio_balloon *vb) > > callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL; > > } > > > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_HINTING)) { > > + names[VIRTIO_BALLOON_VQ_HINTING] = "hinting_vq"; > > + callbacks[VIRTIO_BALLOON_VQ_HINTING] = 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_HINTING)) > > + vb->hinting_vq = vqs[VIRTIO_BALLOON_VQ_HINTING]; > > + > > 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)) { > > @@ -929,12 +1016,24 @@ static int virtballoon_probe(struct virtio_device *vdev) > > if (err) > > goto out_del_balloon_wq; > > } > > + > > + vb->a_dev_info.react = virtballoon_aerator_react; > > + vb->a_dev_info.capacity = VIRTIO_BALLOON_ARRAY_HINTS_MAX; > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_HINTING)) { > > + err = aerator_startup(&vb->a_dev_info); > > + if (err) > > + goto out_unregister_shrinker; > > + } > > + > > virtio_device_ready(vdev); > > > > if (towards_target(vb)) > > virtballoon_changed(vdev); > > return 0; > > > > +out_unregister_shrinker: > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM)) > > + virtio_balloon_unregister_shrinker(vb); > > out_del_balloon_wq: > > if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > > destroy_workqueue(vb->balloon_wq); > > @@ -963,6 +1062,8 @@ static void virtballoon_remove(struct virtio_device *vdev) > > { > > struct virtio_balloon *vb = vdev->priv; > > > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_HINTING)) > > + aerator_shutdown(); > > if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM)) > > virtio_balloon_unregister_shrinker(vb); > > spin_lock_irq(&vb->stop_update_lock); > > @@ -1032,6 +1133,7 @@ static int virtballoon_validate(struct virtio_device *vdev) > > VIRTIO_BALLOON_F_DEFLATE_ON_OOM, > > VIRTIO_BALLOON_F_FREE_PAGE_HINT, > > VIRTIO_BALLOON_F_PAGE_POISON, > > + VIRTIO_BALLOON_F_HINTING, > > }; > > > > 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..2b0f62814e22 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_HINTING 5 /* Page hinting virtqueue */ > > > > /* Size of a PFN in the balloon interface. */ > > #define VIRTIO_BALLOON_PFN_SHIFT 12 > > > > The approach here is very close to what on-demand hinting that is > already upstream does. > > This should have resulted in a most of the code being shared > but this does not seem to happen here. > > Can we unify the code in some way? > It can still use a separate feature flag, but there are things > I like very much about current hinting code, such as > using s/g instead of passing PFNs in a buffer. > > If this doesn't work could you elaborate on why? As far as sending a scatter gather that shouldn't be too much of an issue, however I need to double check that I will still be able to keep the completions as a single block. One significant spot where the "VIRTIO_BALLOON_F_FREE_PAGE_HINT" code and my code differs. My code is processing a fixed discreet block of pages at a time, whereas the FREE_PAGE_HINT code is slurping up all available high-order memory and stuffing it into a giant balloon and has more of a streaming setup as it doesn't return things until either forced to by the shrinker or once it has processed all available memory. The basic idea with the bubble hinting was to essentially create mini balloons. As such I had based the code off of the balloon inflation code. The only spot where it really differs is that I needed the ability to pass higher order pages so I tweaked thinks and passed "hints" instead of "pfns".