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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel