From: David Hildenbrand <david@redhat.com> To: Anshuman Khandual <anshuman.khandual@arm.com>, linux-mm@kvack.org, akpm@linux-foundation.org Cc: linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC V2 1/3] mm/hotplug: Prevalidate the address range being added with platform Date: Wed, 2 Dec 2020 10:20:35 +0100 [thread overview] Message-ID: <864c701a-4391-f768-1b95-1992e21835a2@redhat.com> (raw) In-Reply-To: <1606706992-26656-2-git-send-email-anshuman.khandual@arm.com> On 30.11.20 04:29, Anshuman Khandual wrote: > This introduces memhp_range_allowed() which can be called in various memory > hotplug paths to prevalidate the address range which is being added, with > the platform. Then memhp_range_allowed() calls memhp_get_pluggable_range() > which provides applicable address range depending on whether linear mapping > is required or not. For ranges that require linear mapping, it calls a new > arch callback arch_get_mappable_range() which the platform can override. So > the new callback, in turn provides the platform an opportunity to configure > acceptable memory hotplug address ranges in case there are constraints. > > This mechanism will help prevent platform specific errors deep down during > hotplug calls. This drops now redundant check_hotplug_memory_addressable() > check in __add_pages(). > [...] > /* > * Reasonably generic function for adding memory. It is > * expected that archs that support memory hotplug will > @@ -317,10 +304,6 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages, > if (WARN_ON_ONCE(!params->pgprot.pgprot)) > return -EINVAL; > > - err = check_hotplug_memory_addressable(pfn, nr_pages); > - if (err) > - return err; > - I was wondering if we should add a VM_BUG_ON(!memhp_range_allowed()) here to make it clearer that callers are expected to check that first. Maybe an other places as well (e.g., arch code where we remove the original checks) [...] > #endif /* CONFIG_MEMORY_HOTREMOVE */ > diff --git a/mm/memremap.c b/mm/memremap.c > index 16b2fb482da1..26c1825756cc 100644 > --- a/mm/memremap.c > +++ b/mm/memremap.c > @@ -185,6 +185,7 @@ static void dev_pagemap_percpu_release(struct percpu_ref *ref) > static int pagemap_range(struct dev_pagemap *pgmap, struct mhp_params *params, > int range_id, int nid) > { > + const bool is_private = pgmap->type == MEMORY_DEVICE_PRIVATE; > struct range *range = &pgmap->ranges[range_id]; > struct dev_pagemap *conflict_pgmap; > int error, is_ram; > @@ -230,6 +231,9 @@ static int pagemap_range(struct dev_pagemap *pgmap, struct mhp_params *params, > if (error) > goto err_pfn_remap; > > + if (!memhp_range_allowed(range->start, range_len(range), !is_private)) > + goto err_pfn_remap; > + > mem_hotplug_begin(); > > /* > @@ -243,7 +247,7 @@ static int pagemap_range(struct dev_pagemap *pgmap, struct mhp_params *params, > * the CPU, we do want the linear mapping and thus use > * arch_add_memory(). > */ > - if (pgmap->type == MEMORY_DEVICE_PRIVATE) { > + if (is_private) { > error = add_pages(nid, PHYS_PFN(range->start), > PHYS_PFN(range_len(range)), params); > } else { > In general, LGTM. -- Thanks, David / dhildenb
WARNING: multiple messages have this Message-ID (diff)
From: David Hildenbrand <david@redhat.com> To: Anshuman Khandual <anshuman.khandual@arm.com>, linux-mm@kvack.org, akpm@linux-foundation.org Cc: linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC V2 1/3] mm/hotplug: Prevalidate the address range being added with platform Date: Wed, 2 Dec 2020 10:20:35 +0100 [thread overview] Message-ID: <864c701a-4391-f768-1b95-1992e21835a2@redhat.com> (raw) In-Reply-To: <1606706992-26656-2-git-send-email-anshuman.khandual@arm.com> On 30.11.20 04:29, Anshuman Khandual wrote: > This introduces memhp_range_allowed() which can be called in various memory > hotplug paths to prevalidate the address range which is being added, with > the platform. Then memhp_range_allowed() calls memhp_get_pluggable_range() > which provides applicable address range depending on whether linear mapping > is required or not. For ranges that require linear mapping, it calls a new > arch callback arch_get_mappable_range() which the platform can override. So > the new callback, in turn provides the platform an opportunity to configure > acceptable memory hotplug address ranges in case there are constraints. > > This mechanism will help prevent platform specific errors deep down during > hotplug calls. This drops now redundant check_hotplug_memory_addressable() > check in __add_pages(). > [...] > /* > * Reasonably generic function for adding memory. It is > * expected that archs that support memory hotplug will > @@ -317,10 +304,6 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages, > if (WARN_ON_ONCE(!params->pgprot.pgprot)) > return -EINVAL; > > - err = check_hotplug_memory_addressable(pfn, nr_pages); > - if (err) > - return err; > - I was wondering if we should add a VM_BUG_ON(!memhp_range_allowed()) here to make it clearer that callers are expected to check that first. Maybe an other places as well (e.g., arch code where we remove the original checks) [...] > #endif /* CONFIG_MEMORY_HOTREMOVE */ > diff --git a/mm/memremap.c b/mm/memremap.c > index 16b2fb482da1..26c1825756cc 100644 > --- a/mm/memremap.c > +++ b/mm/memremap.c > @@ -185,6 +185,7 @@ static void dev_pagemap_percpu_release(struct percpu_ref *ref) > static int pagemap_range(struct dev_pagemap *pgmap, struct mhp_params *params, > int range_id, int nid) > { > + const bool is_private = pgmap->type == MEMORY_DEVICE_PRIVATE; > struct range *range = &pgmap->ranges[range_id]; > struct dev_pagemap *conflict_pgmap; > int error, is_ram; > @@ -230,6 +231,9 @@ static int pagemap_range(struct dev_pagemap *pgmap, struct mhp_params *params, > if (error) > goto err_pfn_remap; > > + if (!memhp_range_allowed(range->start, range_len(range), !is_private)) > + goto err_pfn_remap; > + > mem_hotplug_begin(); > > /* > @@ -243,7 +247,7 @@ static int pagemap_range(struct dev_pagemap *pgmap, struct mhp_params *params, > * the CPU, we do want the linear mapping and thus use > * arch_add_memory(). > */ > - if (pgmap->type == MEMORY_DEVICE_PRIVATE) { > + if (is_private) { > error = add_pages(nid, PHYS_PFN(range->start), > PHYS_PFN(range_len(range)), params); > } else { > In general, LGTM. -- Thanks, David / dhildenb _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-12-02 9:22 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-30 3:29 [RFC V2 0/3] mm/hotplug: Pre-validate the address range with platform Anshuman Khandual 2020-11-30 3:29 ` Anshuman Khandual 2020-11-30 3:29 ` [RFC V2 1/3] mm/hotplug: Prevalidate the address range being added " Anshuman Khandual 2020-11-30 3:29 ` Anshuman Khandual 2020-12-02 5:10 ` kernel test robot 2020-12-02 9:20 ` David Hildenbrand [this message] 2020-12-02 9:20 ` David Hildenbrand 2020-12-02 12:15 ` Anshuman Khandual 2020-12-02 12:15 ` Anshuman Khandual 2020-11-30 3:29 ` [RFC V2 2/3] arm64/mm: Define arch_get_mappable_range() Anshuman Khandual 2020-11-30 3:29 ` Anshuman Khandual 2020-11-30 5:29 ` kernel test robot 2020-11-30 17:38 ` kernel test robot 2020-12-02 9:26 ` David Hildenbrand 2020-12-02 9:26 ` David Hildenbrand 2020-12-02 12:17 ` Anshuman Khandual 2020-12-02 12:17 ` Anshuman Khandual 2020-11-30 3:29 ` [RFC V2 3/3] s390/mm: " Anshuman Khandual 2020-11-30 3:29 ` Anshuman Khandual 2020-11-30 5:40 ` kernel test robot 2020-12-02 20:32 ` Heiko Carstens 2020-12-02 20:32 ` Heiko Carstens 2020-12-03 0:33 ` Anshuman Khandual 2020-12-03 0:33 ` Anshuman Khandual 2020-12-03 11:51 ` Heiko Carstens 2020-12-03 11:51 ` Heiko Carstens 2020-12-03 12:01 ` David Hildenbrand 2020-12-03 12:01 ` David Hildenbrand 2020-12-07 4:38 ` Anshuman Khandual 2020-12-07 4:38 ` Anshuman Khandual 2020-12-07 9:03 ` David Hildenbrand 2020-12-07 9:03 ` David Hildenbrand 2020-12-08 5:32 ` Anshuman Khandual 2020-12-08 5:32 ` Anshuman Khandual 2020-12-08 8:38 ` David Hildenbrand 2020-12-08 8:38 ` David Hildenbrand 2020-12-02 6:44 ` [RFC V2 0/3] mm/hotplug: Pre-validate the address range with platform Anshuman Khandual 2020-12-02 6:44 ` Anshuman Khandual 2020-12-02 20:35 ` Heiko Carstens 2020-12-02 20:35 ` Heiko Carstens 2020-12-03 0:12 ` Anshuman Khandual 2020-12-03 0:12 ` Anshuman Khandual
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=864c701a-4391-f768-1b95-1992e21835a2@redhat.com \ --to=david@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=anshuman.khandual@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-s390@vger.kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.