Date:   Mon, 31 Aug 2020 11:35:36 +0200
From:   Pankaj Gupta <pankaj.gupta.linux@...il.com>
To:     David Hildenbrand <david@...hat.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org,
        Linux MM <linux-mm@...ck.org>, linux-hyperv@...r.kernel.org,
        xen-devel@...ts.xenproject.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Kees Cook <keescook@...omium.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Roger Pau Monné <roger.pau@...rix.com>,
        Julien Grall <julien@....org>, Baoquan He <bhe@...hat.com>,
        Wei Yang <richardw.yang@...ux.intel.com>
Subject: Re: [PATCH v1 2/5] kernel/resource: merge_system_ram_resources() to
 merge resources after hotplug

> Some add_memory*() users add memory in small, contiguous memory blocks.
> Examples include virtio-mem, hyper-v balloon, and the XEN balloon.
>
> This can quickly result in a lot of memory resources, whereby the actual
> resource boundaries are not of interest (e.g., it might be relevant for
> DIMMs, exposed via /proc/iomem to user space). We really want to merge
> added resources in this scenario where possible.
>
> Let's provide an interface to trigger merging of applicable child
> resources. It will be, for example, used by virtio-mem to trigger
> merging of system ram resources it added to its resource container, but
> also by XEN and Hyper-V to trigger merging of system ram resources in
> iomem_resource.
>
> Note: We really want to merge after the whole operation succeeded, not
> directly when adding a resource to the resource tree (it would break
> add_memory_resource() and require splitting resources again when the
> operation failed - e.g., due to -ENOMEM).
>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Michal Hocko <mhocko@...e.com>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Jason Gunthorpe <jgg@...pe.ca>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: "K. Y. Srinivasan" <kys@...rosoft.com>
> Cc: Haiyang Zhang <haiyangz@...rosoft.com>
> Cc: Stephen Hemminger <sthemmin@...rosoft.com>
> Cc: Wei Liu <wei.liu@...nel.org>
> Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>
> Cc: Juergen Gross <jgross@...e.com>
> Cc: Stefano Stabellini <sstabellini@...nel.org>
> Cc: Roger Pau Monné <roger.pau@...rix.com>
> Cc: Julien Grall <julien@....org>
> Cc: Pankaj Gupta <pankaj.gupta.linux@...il.com>
> Cc: Baoquan He <bhe@...hat.com>
> Cc: Wei Yang <richardw.yang@...ux.intel.com>
> Signed-off-by: David Hildenbrand <david@...hat.com>
> ---
>  include/linux/ioport.h |  3 +++
>  kernel/resource.c      | 52 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
>
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 52a91f5fa1a36..3bb0020cd6ddc 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -251,6 +251,9 @@ extern void __release_region(struct resource *, resource_size_t,
>  extern void release_mem_region_adjustable(struct resource *, resource_size_t,
>                                           resource_size_t);
>  #endif
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +extern void merge_system_ram_resources(struct resource *res);
> +#endif
>
>  /* Wrappers for managed devices */
>  struct device;
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 1dcef5d53d76e..b4e0963edadd2 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1360,6 +1360,58 @@ void release_mem_region_adjustable(struct resource *parent,
>  }
>  #endif /* CONFIG_MEMORY_HOTREMOVE */
>
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +static bool system_ram_resources_mergeable(struct resource *r1,
> +                                          struct resource *r2)
> +{
> +       return r1->flags == r2->flags && r1->end + 1 == r2->start &&
> +              r1->name == r2->name && r1->desc == r2->desc &&
> +              !r1->child && !r2->child;
> +}
> +
> +/*
> + * merge_system_ram_resources - try to merge contiguous system ram resources
> + * @parent: parent resource descriptor
> + *
> + * This interface is intended for memory hotplug, whereby lots of contiguous
> + * system ram resources are added (e.g., via add_memory*()) by a driver, and
> + * the actual resource boundaries are not of interest (e.g., it might be
> + * relevant for DIMMs). Only immediate child resources that are busy and
> + * don't have any children are considered. All applicable child resources
> + * must be immutable during the request.
> + *
> + * Note:
> + * - The caller has to make sure that no pointers to resources that might
> + *   get merged are held anymore. Callers should only trigger merging of child
> + *   resources when they are the only one adding system ram resources to the
> + *   parent (besides during boot).
> + * - release_mem_region_adjustable() will split on demand on memory hotunplug
> + */
> +void merge_system_ram_resources(struct resource *parent)
> +{
> +       const unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> +       struct resource *cur, *next;
> +
> +       write_lock(&resource_lock);
> +
> +       cur = parent->child;
> +       while (cur && cur->sibling) {
> +               next = cur->sibling;
> +               if ((cur->flags & flags) == flags &&

Maybe this can be changed to:
!(cur->flags & ~flags)

> +                   system_ram_resources_mergeable(cur, next)) {
> +                       cur->end = next->end;
> +                       cur->sibling = next->sibling;
> +                       free_resource(next);
> +                       next = cur->sibling;
> +               }
> +               cur = next;
> +       }
> +
> +       write_unlock(&resource_lock);
> +}
> +EXPORT_SYMBOL(merge_system_ram_resources);
> +#endif /* CONFIG_MEMORY_HOTPLUG */
> +
>  /*
>   * Managed region resource
>   */
> --
> 2.26.2
>

Powered by blists - more mailing lists