Date: Sat, 21 Jun 2014 08:31:21 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Roger Pau Monn? <roger.pau@citrix.com> Cc: virtualization@FreeBSD.org, "freebsd-xen@freebsd.org" <freebsd-xen@freebsd.org>, bryanv@FreeBSD.org Subject: Re: FreeBSD and memory balloon drivers Message-ID: <20140621053121.GK3991@kib.kiev.ua> In-Reply-To: <53A467B4.8070501@citrix.com> References: <53A40079.9000804@citrix.com> <20140620132816.GH3991@kib.kiev.ua> <53A4501F.4020201@citrix.com> <20140620152610.GI3991@kib.kiev.ua> <53A467B4.8070501@citrix.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--N7OK10pvBj1bDp4V
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Jun 20, 2014 at 06:56:20PM +0200, Roger Pau Monn? wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>=20
> On 20/06/14 17:26, Konstantin Belousov wrote:
> > On Fri, Jun 20, 2014 at 05:15:43PM +0200, Roger Pau Monn? wrote:
> >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >>=20
> >> On 20/06/14 15:28, Konstantin Belousov wrote:
> >>> On Fri, Jun 20, 2014 at 11:35:53AM +0200, Roger Pau Monn?
> >>> wrote:
> >>>> Hello,
> >>>>=20
> >>>> I've been looking into the Xen balloon driver, because I've=20
> >>>> experienced problems when ballooning memory down which AFAICT
> >>>> are also present in the VirtIO balloon driver. The problem
> >>>> I've experienced is that when ballooning memory down, we
> >>>> basically allocate a bunch of memory as WIRED, to make sure
> >>>> nobody tries to swap it do disk, since it will crash the
> >>>> kernel because the memory is not populated. Due to this
> >>>> massive amount of memory allocated as WIRED, user-space
> >>>> programs that try to use mlock will fail because we hit the
> >>>> limit in vm.max_wired.
> >>>>=20
> >>>> I'm not sure what's the best way to deal with this
> >>>> limitation, should vm.max_wired be changed from the balloon
> >>>> drivers when ballooning down/up? Is there anyway to remove
> >>>> the pages ballooned down from the memory accounting of wired
> >>>> pages?
> >>>=20
> >>> You could change the type of pages the ballon driver is=20
> >>> allocating. Instead of wired pages, you may request unmanaged,
> >>> by passing NULL object to vm_page_alloc(). This would also
> >>> save on the trie nodes for managing the radix trie for the
> >>> object. There are still plinks or listq to keep track of the
> >>> allocated pages.
> >>=20
> >> Thanks for the info, I have the following patch which fixes the
> >> usage of WIRED for both the Xen and the VirtIO balloon drivers,
> >> could someone please test the VirtIO side?
> > I briefly looked at the xen balloon. You do not need
> > balloon_append(). Use struct vm_page plinks field to link the
> > pages.
>=20
> Sure, thanks for the review, here is an updated version.
It looks fine to me.
>=20
> Roger.
>=20
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.12 (Darwin)
>=20
> iQEcBAEBAgAGBQJTpGe0AAoJEKXZdqUyumTA5acH/jQ+Iwjw+QTwUtsh9ZLs7Gm0
> h7XaCwa/gASEjzcxK2smIBuKA10LUSoulcYkC3cUWXqaPwFega14JcbySBRI06Z1
> 1bU50DL8TPLuQIzrVWZgtA+QsZkwEu/jhijr0AMxTcnm6Wq1LV5QRDoqhD0kRiHu
> kEW1ez832y/0j+oeQAq0aR67zVw7hoIouH9eLaWXS4Vgxz5YQ2Pal1BMAY/OCgua
> xyF7BHia9KsGTKZ9pPUQfQAW5eJrwAxR0AitQjmOwRKtWyRqymZxhYHjLYOgOKQJ
> w93aLRVeG2oo0NNC6a9JD/c64sGHLABg37mnSTRL/v9gTj9I1DcFoid2q0iDGbM=3D
> =3D8nDn
> -----END PGP SIGNATURE-----
> >From e5c898b1b2b734fdc4aa2acf645efd481f09b71d Mon Sep 17 00:00:00 2001
> From: Roger Pau Monne <roger.pau@citrix.com>
> Date: Fri, 20 Jun 2014 16:34:31 +0200
> Subject: [PATCH] xen/virtio: fix balloon drivers to not mark pages as WIR=
ED
>=20
> Prevent the Xen and VirtIO balloon drivers from marking pages as
> wired. This prevents them from increasing the system wired page count,
> which can lead to mlock failing because of hitting the limit in
> vm.max_wired.
>=20
> In the Xen case make sure pages are zeroed before giving them back to
> the hypervisor, or else we might be leaking data. Also remove the
> balloon_{append/retrieve} and link pages directly into the
> ballooned_pages queue using the plinks.q field in the page struct.
>=20
> Sponsored by: Citrix Systems R&D
> Reviewed by: xxx
> Approved by: xxx
>=20
> dev/virtio/balloon/virtio_balloon.c:
> - Don't allocate pages with VM_ALLOC_WIRED.
>=20
> dev/xen/balloon/balloon.c:
> - Don't allocate pages with VM_ALLOC_WIRED.
> - Make sure pages are zeroed before giving them back to the
> hypervisor.
> - Remove the balloon_entry struct and the balloon_{append/retrieve}
> functions and use the page plinks.q entry to link the pages
> directly into the ballooned_pages queue.
> ---
> sys/dev/virtio/balloon/virtio_balloon.c | 4 +-
> sys/dev/xen/balloon/balloon.c | 87 ++++++++-----------------=
------
> 2 files changed, 23 insertions(+), 68 deletions(-)
>=20
> diff --git a/sys/dev/virtio/balloon/virtio_balloon.c b/sys/dev/virtio/bal=
loon/virtio_balloon.c
> index d540099..6d00ef3 100644
> --- a/sys/dev/virtio/balloon/virtio_balloon.c
> +++ b/sys/dev/virtio/balloon/virtio_balloon.c
> @@ -438,8 +438,7 @@ vtballoon_alloc_page(struct vtballoon_softc *sc)
> {
> vm_page_t m;
> =20
> - m =3D vm_page_alloc(NULL, 0, VM_ALLOC_NORMAL | VM_ALLOC_WIRED |
> - VM_ALLOC_NOOBJ);
> + m =3D vm_page_alloc(NULL, 0, VM_ALLOC_NORMAL | VM_ALLOC_NOOBJ);
> if (m !=3D NULL)
> sc->vtballoon_current_npages++;
> =20
> @@ -450,7 +449,6 @@ static void
> vtballoon_free_page(struct vtballoon_softc *sc, vm_page_t m)
> {
> =20
> - vm_page_unwire(m, PQ_INACTIVE);
> vm_page_free(m);
> sc->vtballoon_current_npages--;
> }
> diff --git a/sys/dev/xen/balloon/balloon.c b/sys/dev/xen/balloon/balloon.c
> index fa56c86..0d2bba2 100644
> --- a/sys/dev/xen/balloon/balloon.c
> +++ b/sys/dev/xen/balloon/balloon.c
> @@ -94,13 +94,8 @@ SYSCTL_ULONG(_dev_xen_balloon, OID_AUTO, low_mem, CTLF=
LAG_RD,
> SYSCTL_ULONG(_dev_xen_balloon, OID_AUTO, high_mem, CTLFLAG_RD,
> &bs.balloon_high, 0, "High-mem balloon");
> =20
> -struct balloon_entry {
> - vm_page_t page;
> - STAILQ_ENTRY(balloon_entry) list;
> -};
> -
> /* List of ballooned pages, threaded through the mem_map array. */
> -static STAILQ_HEAD(,balloon_entry) ballooned_pages;
> +static TAILQ_HEAD(,vm_page) ballooned_pages;
> =20
> /* Main work function, always executed in process context. */
> static void balloon_process(void *unused);
> @@ -110,47 +105,6 @@ static void balloon_process(void *unused);
> #define WPRINTK(fmt, args...) \
> printk(KERN_WARNING "xen_mem: " fmt, ##args)
> =20
> -/* balloon_append: add the given page to the balloon. */
> -static int
> -balloon_append(vm_page_t page)
> -{
> - struct balloon_entry *entry;
> -
> - mtx_assert(&balloon_mutex, MA_OWNED);
> -
> - entry =3D malloc(sizeof(struct balloon_entry), M_BALLOON, M_NOWAIT);
> - if (!entry)
> - return (ENOMEM);
> - entry->page =3D page;
> - STAILQ_INSERT_HEAD(&ballooned_pages, entry, list);
> - bs.balloon_low++;
> -
> - return (0);
> -}
> -
> -/* balloon_retrieve: rescue a page from the balloon, if it is not empty.=
*/
> -static vm_page_t
> -balloon_retrieve(void)
> -{
> - vm_page_t page;
> - struct balloon_entry *entry;
> -
> - mtx_assert(&balloon_mutex, MA_OWNED);
> -
> - if (STAILQ_EMPTY(&ballooned_pages))
> - return (NULL);
> -
> - entry =3D STAILQ_FIRST(&ballooned_pages);
> - STAILQ_REMOVE_HEAD(&ballooned_pages, list);
> -
> - page =3D entry->page;
> - free(entry, M_BALLOON);
> -=09
> - bs.balloon_low--;
> -
> - return (page);
> -}
> -
> static unsigned long=20
> current_target(void)
> {
> @@ -203,7 +157,6 @@ static int
> increase_reservation(unsigned long nr_pages)
> {
> unsigned long pfn, i;
> - struct balloon_entry *entry;
> vm_page_t page;
> long rc;
> struct xen_memory_reservation reservation =3D {
> @@ -217,10 +170,9 @@ increase_reservation(unsigned long nr_pages)
> if (nr_pages > nitems(frame_list))
> nr_pages =3D nitems(frame_list);
> =20
> - for (entry =3D STAILQ_FIRST(&ballooned_pages), i =3D 0;
> - i < nr_pages; i++, entry =3D STAILQ_NEXT(entry, list)) {
> - KASSERT(entry, ("ballooned_pages list corrupt"));
> - page =3D entry->page;
> + for (page =3D TAILQ_FIRST(&ballooned_pages), i =3D 0;
> + i < nr_pages; i++, page =3D TAILQ_NEXT(page, plinks.q)) {
> + KASSERT(page !=3D NULL, ("ballooned_pages list corrupt"));
> frame_list[i] =3D (VM_PAGE_TO_PHYS(page) >> PAGE_SHIFT);
> }
> =20
> @@ -245,8 +197,10 @@ increase_reservation(unsigned long nr_pages)
> }
> =20
> for (i =3D 0; i < nr_pages; i++) {
> - page =3D balloon_retrieve();
> - KASSERT(page, ("balloon_retrieve failed"));
> + page =3D TAILQ_FIRST(&ballooned_pages);
> + KASSERT(page !=3D NULL, ("Unable to get ballooned page"));
> + TAILQ_REMOVE(&ballooned_pages, page, plinks.q);
> + bs.balloon_low--;
> =20
> pfn =3D (VM_PAGE_TO_PHYS(page) >> PAGE_SHIFT);
> KASSERT((xen_feature(XENFEAT_auto_translated_physmap) ||
> @@ -255,7 +209,6 @@ increase_reservation(unsigned long nr_pages)
> =20
> set_phys_to_machine(pfn, frame_list[i]);
> =20
> - vm_page_unwire(page, PQ_INACTIVE);
> vm_page_free(page);
> }
> =20
> @@ -286,24 +239,27 @@ decrease_reservation(unsigned long nr_pages)
> for (i =3D 0; i < nr_pages; i++) {
> if ((page =3D vm_page_alloc(NULL, 0,=20
> VM_ALLOC_NORMAL | VM_ALLOC_NOOBJ |=20
> - VM_ALLOC_WIRED | VM_ALLOC_ZERO)) =3D=3D NULL) {
> + VM_ALLOC_ZERO)) =3D=3D NULL) {
> nr_pages =3D i;
> need_sleep =3D 1;
> break;
> }
> =20
> + if ((page->flags & PG_ZERO) =3D=3D 0) {
> + /*
> + * Zero the page, or else we might be leaking
> + * important data to other domains on the same
> + * host.
> + */
> + pmap_zero_page(page);
> + }
> +
> pfn =3D (VM_PAGE_TO_PHYS(page) >> PAGE_SHIFT);
> frame_list[i] =3D PFNTOMFN(pfn);
> =20
> set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
> - if (balloon_append(page) !=3D 0) {
> - vm_page_unwire(page, PQ_INACTIVE);
> - vm_page_free(page);
> -
> - nr_pages =3D i;
> - need_sleep =3D 1;
> - break;
> - }
> + TAILQ_INSERT_HEAD(&ballooned_pages, page, plinks.q);
> + bs.balloon_low++;
> }
> =20
> set_xen_guest_handle(reservation.extent_start, frame_list);
> @@ -438,7 +394,8 @@ balloon_init(void *arg)
> /* Initialise the balloon with excess memory space. */
> for (pfn =3D xen_start_info->nr_pages; pfn < max_pfn; pfn++) {
> page =3D PHYS_TO_VM_PAGE(pfn << PAGE_SHIFT);
> - balloon_append(page);
> + TAILQ_INSERT_HEAD(&ballooned_pages, page, plinks.q);
> + bs.balloon_low++;
> }
> #undef max_pfn
> #endif
> --=20
> 1.7.7.5 (Apple Git-26)
>=20
--N7OK10pvBj1bDp4V
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBAgAGBQJTpRioAAoJEJDCuSvBvK1BSxEP/1Oxdr7ActnPpw7M6pRHqXMe
63ZkiIdwovfBdABA/Qs/+EAgCuPxbZmaD3e3b/1ypi/qXihB4kbnWTTneXUei5NF
7Xot0YppkCmnKLiGiku3rV/biIFjmTJg5aCsrKdTpQfHCG1WInDwWZTW4UlMwkf2
j05qqBY6CeI8ckvmGDDZPpVAtnUnMI0AQOV2hve77kN3zWisxHG3qsBpCFIsDLAk
9eNBKasKHVWZlDTj1SHT9Fk3UgEqNOoSdOPgeMA+RMNAerLlg6uQXkdFZL8+nCA9
KzChvmglVepdC46qwai2UgkCr7ikEy6btijb4d+3pr0+XbzlfOCJ5nduwaYN72Uz
7qgxpX3AgagBIdLfv1zBtWX7K1fbQVkCLIe0pwPqKw8fAUP1lu2TmjsU+Ziq6wLa
ncN0Lafh3sihUt6rsrW2GGFTtXiWqJqtb2QHdLYgxA7F27OupxLXl+ZXFgqTHSdc
BXOFsE6NxtGkf8U9dnNHgHVDDyXKTMzy8/VblRxw2VIBSPV4ilCOXuE8E0+3/ASg
mFZv2o8Yr7SO5OHouZAdKYvtW29Ch4YYl1HvJJt+ZHgZmvGQiv9HUqzPJvgOGrKK
tNth+Vcbp14xEhgHrnuJeymtAa39hhBXJVvgOQJRJsWpyhhycLCAaIgr8h0qC8A/
Ax1qZuC61xLOmFWRHPMf
=fFv2
-----END PGP SIGNATURE-----
--N7OK10pvBj1bDp4V--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140621053121.GK3991>