From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: David Hildenbrand <david@redhat.com>,
Roger Pau Monne <roger.pau@citrix.com>,
linux-kernel@vger.kernel.org
Cc: Juergen Gross <jgross@suse.com>,
Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>,
David Airlie <airlied@linux.ie>,
Yan Yankovskyi <yyankovskyi@gmail.com>,
dri-devel@lists.freedesktop.org, Michal Hocko <mhocko@kernel.org>,
linux-mm@kvack.org, Daniel Vetter <daniel@ffwll.ch>,
xen-devel@lists.xenproject.org,
Dan Williams <dan.j.williams@intel.com>,
Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH v2 4/4] xen: add helpers to allocate unpopulated memory
Date: Fri, 24 Jul 2020 12:36:33 -0400 [thread overview]
Message-ID: <1fd1d29e-5c10-0c29-0628-b79807f81de6@oracle.com> (raw)
In-Reply-To: <1778c97f-3a69-8280-141c-879814dd213f@redhat.com>
On 7/24/20 10:34 AM, David Hildenbrand wrote:
> CCing Dan
>
> On 24.07.20 14:42, Roger Pau Monne wrote:
>> 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 @@
>> + */
>> +
>> +#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)
Less generic names? How about list_lock, pg_list, pg_count,
fill_pglist()? (But these are bad too, so maybe you can come up with
something better)
>> +{
>> + 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";
Typically iomem resources only capitalize first letters.
>> + 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);
Are we not going to end up with a whole bunch of "Xen scratch" resource
ranges for each miss in the page list? Or do we expect them to get merged?
>> + if (ret < 0) {
>> + pr_err("Cannot allocate new IOMEM resource\n");
>> + kfree(pgmap);
>> + return ret;
>> + }
>> +
>> + nid = memory_add_physaddr_to_nid(pgmap->res.start);
Should we consider page range crossing node boundaries?
>> +
>> +#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);
>> +
>> + /*
>> + * 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);
(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);
>> +
>> + 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);
>> +
>> +
>> +#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).
>> + */
This loop will be executing only for PV guests so we can just bail out
for non-PV guests here.
-boris
>> + 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);
next prev parent reply other threads:[~2020-07-24 16:37 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-24 12:42 [PATCH v2 0/4] xen/balloon: fixes for memory hotplug Roger Pau Monne
2020-07-24 12:42 ` [PATCH v2 1/4] xen/balloon: fix accounting in alloc_xenballooned_pages error path Roger Pau Monne
2020-07-24 13:11 ` Jürgen Groß
2020-07-24 12:42 ` [PATCH v2 2/4] xen/balloon: make the balloon wait interruptible Roger Pau Monne
2020-07-24 13:13 ` Jürgen Groß
2020-07-24 12:42 ` [PATCH v2 3/4] Revert "xen/balloon: Fix crash when ballooning on x86 32 bit PAE" Roger Pau Monne
2020-07-24 13:20 ` Jürgen Groß
2020-07-24 12:42 ` [PATCH v2 4/4] xen: add helpers to allocate unpopulated memory Roger Pau Monne
2020-07-24 14:33 ` Jürgen Groß
2020-07-24 14:34 ` David Hildenbrand
2020-07-24 16:36 ` Boris Ostrovsky [this message]
2020-07-27 8:00 ` David Hildenbrand
2020-07-27 8:42 ` Roger Pau Monné
2020-07-24 15:46 ` kernel test robot
2020-07-25 2:48 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1fd1d29e-5c10-0c29-0628-b79807f81de6@oracle.com \
--to=boris.ostrovsky@oracle.com \
--cc=airlied@linux.ie \
--cc=dan.carpenter@oracle.com \
--cc=dan.j.williams@intel.com \
--cc=daniel@ffwll.ch \
--cc=david@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=jgross@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=oleksandr_andrushchenko@epam.com \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
--cc=yyankovskyi@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).