lkml.org 
[lkml]   [2020]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 4/4] xen: add helpers to allocate unpopulated memory
    From
    Date
    On 24.07.20 14:42, Roger Pau Monne wrote:
    > To be used in order to create foreign mappings. This is based on the
    > ZONE_DEVICE facility which is used by persistent memory devices in
    > order to create struct pages and kernel virtual mappings for the IOMEM
    > areas of such devices. Note that on kernels without support for
    > ZONE_DEVICE Xen will fallback to use ballooned pages in order to
    > create foreign mappings.
    >
    > The newly added helpers use the same parameters as the existing
    > {alloc/free}_xenballooned_pages functions, which allows for in-place
    > replacement of the callers. Once a memory region has been added to be
    > used as scratch mapping space it will no longer be released, and pages
    > returned are kept in a linked list. This allows to have a buffer of
    > pages and prevents resorting to frequent additions and removals of
    > regions.
    >
    > If enabled (because ZONE_DEVICE is supported) the usage of the new
    > functionality untangles Xen balloon and RAM hotplug from the usage of
    > unpopulated physical memory ranges to map foreign pages, which is the
    > correct thing to do in order to avoid mappings of foreign pages depend
    > on memory hotplug.
    >
    > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
    > ---
    > I've not added a new memory_type type and just used
    > MEMORY_DEVICE_DEVDAX which seems to be what we want for such memory
    > regions. I'm unsure whether abusing this type is fine, or if I should
    > instead add a specific type, maybe MEMORY_DEVICE_GENERIC? I don't
    > think we should be using a specific Xen type at all.
    > ---
    > Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
    > Cc: David Airlie <airlied@linux.ie>
    > Cc: Daniel Vetter <daniel@ffwll.ch>
    > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
    > Cc: Juergen Gross <jgross@suse.com>
    > Cc: Stefano Stabellini <sstabellini@kernel.org>
    > Cc: Dan Carpenter <dan.carpenter@oracle.com>
    > Cc: Roger Pau Monne <roger.pau@citrix.com>
    > Cc: Wei Liu <wl@xen.org>
    > Cc: Yan Yankovskyi <yyankovskyi@gmail.com>
    > Cc: dri-devel@lists.freedesktop.org
    > Cc: xen-devel@lists.xenproject.org
    > Cc: linux-mm@kvack.org
    > Cc: David Hildenbrand <david@redhat.com>
    > Cc: Michal Hocko <mhocko@kernel.org>
    > ---
    > drivers/gpu/drm/xen/xen_drm_front_gem.c | 8 +-
    > drivers/xen/Makefile | 1 +
    > drivers/xen/balloon.c | 4 +-
    > drivers/xen/grant-table.c | 4 +-
    > drivers/xen/privcmd.c | 4 +-
    > drivers/xen/unpopulated-alloc.c | 222 ++++++++++++++++++++++++
    > drivers/xen/xenbus/xenbus_client.c | 6 +-
    > drivers/xen/xlate_mmu.c | 4 +-
    > include/xen/xen.h | 8 +
    > 9 files changed, 246 insertions(+), 15 deletions(-)
    > create mode 100644 drivers/xen/unpopulated-alloc.c
    >
    > diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
    > index f0b85e094111..9dd06eae767a 100644
    > --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
    > +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
    > @@ -99,8 +99,8 @@ static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
    > * allocate ballooned pages which will be used to map
    > * grant references provided by the backend
    > */
    > - ret = alloc_xenballooned_pages(xen_obj->num_pages,
    > - xen_obj->pages);
    > + ret = xen_alloc_unpopulated_pages(xen_obj->num_pages,
    > + xen_obj->pages);
    > if (ret < 0) {
    > DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
    > xen_obj->num_pages, ret);
    > @@ -152,8 +152,8 @@ void xen_drm_front_gem_free_object_unlocked(struct drm_gem_object *gem_obj)
    > } else {
    > if (xen_obj->pages) {
    > if (xen_obj->be_alloc) {
    > - free_xenballooned_pages(xen_obj->num_pages,
    > - xen_obj->pages);
    > + xen_free_unpopulated_pages(xen_obj->num_pages,
    > + xen_obj->pages);
    > gem_free_pages_array(xen_obj);
    > } else {
    > drm_gem_put_pages(&xen_obj->base,
    > diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
    > index 0d322f3d90cd..788a5d9c8ef0 100644
    > --- a/drivers/xen/Makefile
    > +++ b/drivers/xen/Makefile
    > @@ -42,3 +42,4 @@ xen-gntdev-$(CONFIG_XEN_GNTDEV_DMABUF) += gntdev-dmabuf.o
    > xen-gntalloc-y := gntalloc.o
    > xen-privcmd-y := privcmd.o privcmd-buf.o
    > obj-$(CONFIG_XEN_FRONT_PGDIR_SHBUF) += xen-front-pgdir-shbuf.o
    > +obj-$(CONFIG_ZONE_DEVICE) += unpopulated-alloc.o
    > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
    > index b1d8b028bf80..815ef10eb2ff 100644
    > --- a/drivers/xen/balloon.c
    > +++ b/drivers/xen/balloon.c
    > @@ -654,7 +654,7 @@ void free_xenballooned_pages(int nr_pages, struct page **pages)
    > }
    > EXPORT_SYMBOL(free_xenballooned_pages);
    >
    > -#ifdef CONFIG_XEN_PV
    > +#if defined(CONFIG_XEN_PV) && !defined(CONFIG_ZONE_DEVICE)
    > static void __init balloon_add_region(unsigned long start_pfn,
    > unsigned long pages)
    > {
    > @@ -708,7 +708,7 @@ static int __init balloon_init(void)
    > register_sysctl_table(xen_root);
    > #endif
    >
    > -#ifdef CONFIG_XEN_PV
    > +#if defined(CONFIG_XEN_PV) && !defined(CONFIG_ZONE_DEVICE)
    > {
    > int i;
    >
    > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
    > index 8d06bf1cc347..523dcdf39cc9 100644
    > --- a/drivers/xen/grant-table.c
    > +++ b/drivers/xen/grant-table.c
    > @@ -801,7 +801,7 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
    > {
    > int ret;
    >
    > - ret = alloc_xenballooned_pages(nr_pages, pages);
    > + ret = xen_alloc_unpopulated_pages(nr_pages, pages);
    > if (ret < 0)
    > return ret;
    >
    > @@ -836,7 +836,7 @@ EXPORT_SYMBOL_GPL(gnttab_pages_clear_private);
    > void gnttab_free_pages(int nr_pages, struct page **pages)
    > {
    > gnttab_pages_clear_private(nr_pages, pages);
    > - free_xenballooned_pages(nr_pages, pages);
    > + xen_free_unpopulated_pages(nr_pages, pages);
    > }
    > EXPORT_SYMBOL_GPL(gnttab_free_pages);
    >
    > diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
    > index a250d118144a..56000ab70974 100644
    > --- a/drivers/xen/privcmd.c
    > +++ b/drivers/xen/privcmd.c
    > @@ -425,7 +425,7 @@ static int alloc_empty_pages(struct vm_area_struct *vma, int numpgs)
    > if (pages == NULL)
    > return -ENOMEM;
    >
    > - rc = alloc_xenballooned_pages(numpgs, pages);
    > + rc = xen_alloc_unpopulated_pages(numpgs, pages);
    > if (rc != 0) {
    > pr_warn("%s Could not alloc %d pfns rc:%d\n", __func__,
    > numpgs, rc);
    > @@ -900,7 +900,7 @@ static void privcmd_close(struct vm_area_struct *vma)
    >
    > rc = xen_unmap_domain_gfn_range(vma, numgfns, pages);
    > if (rc == 0)
    > - free_xenballooned_pages(numpgs, pages);
    > + xen_free_unpopulated_pages(numpgs, pages);
    > else
    > pr_crit("unable to unmap MFN range: leaking %d pages. rc=%d\n",
    > numpgs, rc);
    > diff --git a/drivers/xen/unpopulated-alloc.c b/drivers/xen/unpopulated-alloc.c
    > new file mode 100644
    > index 000000000000..aaa91cefbbf9
    > --- /dev/null
    > +++ b/drivers/xen/unpopulated-alloc.c
    > @@ -0,0 +1,222 @@
    > +/*
    > + * Helpers to allocate unpopulated memory for foreign mappings
    > + *
    > + * Copyright (c) 2020, Citrix Systems R&D
    > + *
    > + * This program is free software; you can redistribute it and/or
    > + * modify it under the terms of the GNU General Public License version 2
    > + * as published by the Free Software Foundation; or, when distributed
    > + * separately from the Linux kernel or incorporated into other
    > + * software packages, subject to the following license:
    > + *
    > + * Permission is hereby granted, free of charge, to any person obtaining a copy
    > + * of this source file (the "Software"), to deal in the Software without
    > + * restriction, including without limitation the rights to use, copy, modify,
    > + * merge, publish, distribute, sublicense, and/or sell copies of the Software,
    > + * and to permit persons to whom the Software is furnished to do so, subject to
    > + * the following conditions:
    > + *
    > + * The above copyright notice and this permission notice shall be included in
    > + * all copies or substantial portions of the Software.
    > + *
    > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
    > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
    > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
    > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
    > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
    > + * IN THE SOFTWARE.
    > + */

    Please use:

    // SPDX-License-Identifier: GPL-2.0-only

    instead of the long GPL sermon.

    > +
    > +#include <linux/errno.h>
    > +#include <linux/gfp.h>
    > +#include <linux/kernel.h>
    > +#include <linux/mm.h>
    > +#include <linux/memremap.h>
    > +#include <linux/slab.h>
    > +
    > +#include <asm/page.h>
    > +
    > +#include <xen/page.h>
    > +#include <xen/xen.h>
    > +
    > +static DEFINE_MUTEX(lock);
    > +static LIST_HEAD(list);
    > +static unsigned int count;
    > +
    > +static int fill(unsigned int nr_pages)
    > +{
    > + struct dev_pagemap *pgmap;
    > + void *vaddr;
    > + unsigned int i, alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
    > + int nid, ret;
    > +
    > + pgmap = kzalloc(sizeof(*pgmap), GFP_KERNEL);
    > + if (!pgmap)
    > + return -ENOMEM;
    > +
    > + pgmap->type = MEMORY_DEVICE_DEVDAX;
    > + pgmap->res.name = "XEN SCRATCH";
    > + pgmap->res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
    > +
    > + ret = allocate_resource(&iomem_resource, &pgmap->res,
    > + alloc_pages * PAGE_SIZE, 0, -1,
    > + PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
    > + if (ret < 0) {
    > + pr_err("Cannot allocate new IOMEM resource\n");
    > + kfree(pgmap);
    > + return ret;
    > + }
    > +
    > + nid = memory_add_physaddr_to_nid(pgmap->res.start);
    > +
    > +#ifdef CONFIG_XEN_HAVE_PVMMU
    > + /*
    > + * We don't support PV MMU when Linux and Xen is using
    > + * different page granularity.
    > + */
    > + BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

    Drop that, please. PV MMU is x86 only and we surely don't want to add
    it to another architecture. On x86 this will never trigger.

    > +
    > + /*
    > + * memremap will build page tables for the new memory so
    > + * the p2m must contain invalid entries so the correct
    > + * non-present PTEs will be written.
    > + *
    > + * If a failure occurs, the original (identity) p2m entries
    > + * are not restored since this region is now known not to
    > + * conflict with any devices.
    > + */
    > + if (!xen_feature(XENFEAT_auto_translated_physmap)) {
    > + xen_pfn_t pfn = PFN_DOWN(pgmap->res.start);
    > +
    > + for (i = 0; i < alloc_pages; i++) {
    > + if (!set_phys_to_machine(pfn + i, INVALID_P2M_ENTRY)) {
    > + pr_warn("set_phys_to_machine() failed, no memory added\n");
    > + release_resource(&pgmap->res);
    > + kfree(pgmap);
    > + return -ENOMEM;
    > + }
    > + }
    > + }
    > +#endif
    > +
    > + vaddr = memremap_pages(pgmap, nid);
    > + if (IS_ERR(vaddr)) {
    > + pr_err("Cannot remap memory range\n");
    > + release_resource(&pgmap->res);
    > + kfree(pgmap);
    > + return PTR_ERR(vaddr);
    > + }
    > +
    > + for (i = 0; i < alloc_pages; i++) {
    > + struct page *pg = virt_to_page(vaddr + PAGE_SIZE * i);
    > +
    > + BUG_ON(!virt_addr_valid(vaddr + PAGE_SIZE * i));
    > + list_add(&pg->lru, &list);
    > + count++;
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * xen_alloc_unpopulated_pages - alloc unpopulated pages
    > + * @nr_pages: Number of pages
    > + * @pages: pages returned
    > + * @return 0 on success, error otherwise
    > + */
    > +int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages)
    > +{
    > + unsigned int i;
    > + int ret = 0;
    > +
    > + mutex_lock(&lock);
    > + if (count < nr_pages) {
    > + ret = fill(nr_pages);

    I'd rather use: ret = fill(nr_pages - count);

    > + if (ret)
    > + goto out;
    > + }
    > +
    > + for (i = 0; i < nr_pages; i++) {
    > + struct page *pg = list_first_entry_or_null(&list, struct page,
    > + lru);
    > +
    > + BUG_ON(!pg);
    > + list_del(&pg->lru);
    > + count--;
    > + pages[i] = pg;
    > +
    > +#ifdef CONFIG_XEN_HAVE_PVMMU
    > + /*
    > + * We don't support PV MMU when Linux and Xen is using
    > + * different page granularity.
    > + */
    > + BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

    Having two identical BUILD_BUG_ON() in the same source is really not
    wanted.

    > +
    > + if (!xen_feature(XENFEAT_auto_translated_physmap)) {
    > + ret = xen_alloc_p2m_entry(page_to_pfn(pg));
    > + if (ret < 0) {
    > + unsigned int j;
    > +
    > + for (j = 0; j <= i; j++) {
    > + list_add(&pages[j]->lru, &list);
    > + count++;
    > + }
    > + goto out;
    > + }
    > + }
    > +#endif
    > + }
    > +
    > +out:
    > + mutex_unlock(&lock);
    > + return ret;
    > +}
    > +EXPORT_SYMBOL(xen_alloc_unpopulated_pages);
    > +
    > +/**
    > + * xen_free_unpopulated_pages - return unpopulated pages
    > + * @nr_pages: Number of pages
    > + * @pages: pages to return
    > + */
    > +void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages)
    > +{
    > + unsigned int i;
    > +
    > + mutex_lock(&lock);
    > + for (i = 0; i < nr_pages; i++) {
    > + list_add(&pages[i]->lru, &list);
    > + count++;
    > + }
    > + mutex_unlock(&lock);
    > +}
    > +EXPORT_SYMBOL(xen_free_unpopulated_pages);
    > +
    > +#ifdef CONFIG_XEN_PV
    > +static int __init init(void)
    > +{
    > + unsigned int i;
    > +
    > + if (!xen_domain())
    > + return -ENODEV;
    > +
    > + /*
    > + * Initialize with pages from the extra memory regions (see
    > + * arch/x86/xen/setup.c).
    > + */
    > + for (i = 0; i < XEN_EXTRA_MEM_MAX_REGIONS; i++) {
    > + unsigned int j;
    > +
    > + for (j = 0; j < xen_extra_mem[i].n_pfns; j++) {
    > + struct page *pg =
    > + pfn_to_page(xen_extra_mem[i].start_pfn + j);
    > +
    > + list_add(&pg->lru, &list);
    > + count++;
    > + }
    > + }
    > +
    > + return 0;
    > +}
    > +subsys_initcall(init);
    > +#endif
    > diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
    > index 786fbb7d8be0..70b6c4780fbd 100644
    > --- a/drivers/xen/xenbus/xenbus_client.c
    > +++ b/drivers/xen/xenbus/xenbus_client.c
    > @@ -615,7 +615,7 @@ static int xenbus_map_ring_hvm(struct xenbus_device *dev,
    > bool leaked = false;
    > unsigned int nr_pages = XENBUS_PAGES(nr_grefs);
    >
    > - err = alloc_xenballooned_pages(nr_pages, node->hvm.pages);
    > + err = xen_alloc_unpopulated_pages(nr_pages, node->hvm.pages);
    > if (err)
    > goto out_err;
    >
    > @@ -656,7 +656,7 @@ static int xenbus_map_ring_hvm(struct xenbus_device *dev,
    > addr, nr_pages);
    > out_free_ballooned_pages:
    > if (!leaked)
    > - free_xenballooned_pages(nr_pages, node->hvm.pages);
    > + xen_free_unpopulated_pages(nr_pages, node->hvm.pages);
    > out_err:
    > return err;
    > }
    > @@ -852,7 +852,7 @@ static int xenbus_unmap_ring_hvm(struct xenbus_device *dev, void *vaddr)
    > info.addrs);
    > if (!rv) {
    > vunmap(vaddr);
    > - free_xenballooned_pages(nr_pages, node->hvm.pages);
    > + xen_free_unpopulated_pages(nr_pages, node->hvm.pages);
    > }
    > else
    > WARN(1, "Leaking %p, size %u page(s)\n", vaddr, nr_pages);
    > diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
    > index 7b1077f0abcb..34742c6e189e 100644
    > --- a/drivers/xen/xlate_mmu.c
    > +++ b/drivers/xen/xlate_mmu.c
    > @@ -232,7 +232,7 @@ int __init xen_xlate_map_ballooned_pages(xen_pfn_t **gfns, void **virt,
    > kfree(pages);
    > return -ENOMEM;
    > }
    > - rc = alloc_xenballooned_pages(nr_pages, pages);
    > + rc = xen_alloc_unpopulated_pages(nr_pages, pages);
    > if (rc) {
    > pr_warn("%s Couldn't balloon alloc %ld pages rc:%d\n", __func__,
    > nr_pages, rc);
    > @@ -249,7 +249,7 @@ int __init xen_xlate_map_ballooned_pages(xen_pfn_t **gfns, void **virt,
    > if (!vaddr) {
    > pr_warn("%s Couldn't map %ld pages rc:%d\n", __func__,
    > nr_pages, rc);
    > - free_xenballooned_pages(nr_pages, pages);
    > + xen_free_unpopulated_pages(nr_pages, pages);
    > kfree(pages);
    > kfree(pfns);
    > return -ENOMEM;
    > diff --git a/include/xen/xen.h b/include/xen/xen.h
    > index 19a72f591e2b..aa33bc0d933c 100644
    > --- a/include/xen/xen.h
    > +++ b/include/xen/xen.h
    > @@ -52,4 +52,12 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
    > extern u64 xen_saved_max_mem_size;
    > #endif
    >
    > +#ifdef CONFIG_ZONE_DEVICE
    > +int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages);
    > +void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);
    > +#else
    > +#define xen_alloc_unpopulated_pages alloc_xenballooned_pages
    > +#define xen_free_unpopulated_pages free_xenballooned_pages
    > +#endif
    > +
    > #endif /* _XEN_XEN_H */
    >

    Juergen

    \
     
     \ /
      Last update: 2020-07-24 16:33    [W:4.117 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site