On 24.07.20 18:36, Boris Ostrovsky wrote:
> 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?
>
AFAIK, no resources will get merged (and it's in the general case not
safe to do). The old approach (add_memory_resource()) will end up with
the same situation ("Xen Scratch" vs. "System RAM") one new resource per
added memory block/section.
FWIW, I am looking into merging selected resources in the context of
virtio-mem _after_ adding succeeded (not directly when adding the
resource to the tree). Interface might look something like
void merge_child_mem_resources(struct resource *parent, const char *name);
So I can, for example, trigger merging of all "System RAM (virtio_mem)"
resources, that are located under a device node (e.g., "virtio0").
I also thought about tagging each mergeable resource via something like
"IORESOURCE_MERGEABLE" - whereby the user agrees that it does not hold
any pointers to such a resource. But I don't see yet a copelling reason
to sacrifice space for a new flag.
So with this in place, this code could call once adding succeeded
merge_child_mem_resources(&iomem_resource, "Xen Scratch");
--
Thanks,
David / dhildenb
