| Message ID | 20200916073041.10355-1-david@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | kernel/resource: make iomem_resource implicit in release_mem_region_adjustable() | expand |
On Wed, Sep 16, 2020 at 09:30:41AM +0200, David Hildenbrand wrote: >"mem" in the name already indicates the root, similar to >release_mem_region() and devm_request_mem_region(). Make it implicit. >The only single caller always passes iomem_resource, other parents are >not applicable. > Looks good to me. Reviewed-by: Wei Yang <richard.weiyang@linux.alibaba.com> >Suggested-by: Wei Yang <richard.weiyang@linux.alibaba.com> >Cc: Andrew Morton <akpm@linux-foundation.org> >Cc: Michal Hocko <mhocko@suse.com> >Cc: Dan Williams <dan.j.williams@intel.com> >Cc: Jason Gunthorpe <jgg@ziepe.ca> >Cc: Kees Cook <keescook@chromium.org> >Cc: Ard Biesheuvel <ardb@kernel.org> >Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com> >Cc: Baoquan He <bhe@redhat.com> >Cc: Wei Yang <richard.weiyang@linux.alibaba.com> >Signed-off-by: David Hildenbrand <david@redhat.com> >--- > >Based on next-20200915. Follow up on > "[PATCH v4 0/8] selective merging of system ram resources" [1] >That's in next-20200915. As noted during review of v2 by Wei [2]. > >[1] https://lkml.kernel.org/r/20200911103459.10306-1-david@redhat.com >[2] https://lkml.kernel.org/r/20200915021012.GC2007@L-31X9LVDL-1304.local > >--- > include/linux/ioport.h | 3 +-- > kernel/resource.c | 5 ++--- > mm/memory_hotplug.c | 2 +- > 3 files changed, 4 insertions(+), 6 deletions(-) > >diff --git a/include/linux/ioport.h b/include/linux/ioport.h >index 7e61389dcb01..5135d4b86cd6 100644 >--- a/include/linux/ioport.h >+++ b/include/linux/ioport.h >@@ -251,8 +251,7 @@ extern struct resource * __request_region(struct resource *, > extern void __release_region(struct resource *, resource_size_t, > resource_size_t); > #ifdef CONFIG_MEMORY_HOTREMOVE >-extern void release_mem_region_adjustable(struct resource *, resource_size_t, >- resource_size_t); >+extern void release_mem_region_adjustable(resource_size_t, resource_size_t); > #endif > #ifdef CONFIG_MEMORY_HOTPLUG > extern void merge_system_ram_resource(struct resource *res); >diff --git a/kernel/resource.c b/kernel/resource.c >index 7a91b935f4c2..ca2a666e4317 100644 >--- a/kernel/resource.c >+++ b/kernel/resource.c >@@ -1240,7 +1240,6 @@ EXPORT_SYMBOL(__release_region); > #ifdef CONFIG_MEMORY_HOTREMOVE > /** > * release_mem_region_adjustable - release a previously reserved memory region >- * @parent: parent resource descriptor > * @start: resource start address > * @size: resource region size > * >@@ -1258,9 +1257,9 @@ EXPORT_SYMBOL(__release_region); > * assumes that all children remain in the lower address entry for > * simplicity. Enhance this logic when necessary. > */ >-void release_mem_region_adjustable(struct resource *parent, >- resource_size_t start, resource_size_t size) >+void release_mem_region_adjustable(resource_size_t start, resource_size_t size) > { >+ struct resource *parent = &iomem_resource; > struct resource *new_res = NULL; > bool alloc_nofail = false; > struct resource **p; >diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >index 553c718226b3..7c5e4744ac51 100644 >--- a/mm/memory_hotplug.c >+++ b/mm/memory_hotplug.c >@@ -1764,7 +1764,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) > memblock_remove(start, size); > } > >- release_mem_region_adjustable(&iomem_resource, start, size); >+ release_mem_region_adjustable(start, size); > > try_offline_node(nid); > >-- >2.26.2
On 16.09.20 12:02, Wei Yang wrote: > On Wed, Sep 16, 2020 at 09:30:41AM +0200, David Hildenbrand wrote: >> "mem" in the name already indicates the root, similar to >> release_mem_region() and devm_request_mem_region(). Make it implicit. >> The only single caller always passes iomem_resource, other parents are >> not applicable. >> > > Looks good to me. > > Reviewed-by: Wei Yang <richard.weiyang@linux.alibaba.com> > Thanks for the review!
On Wed, Sep 16, 2020 at 12:03:20PM +0200, David Hildenbrand wrote: >On 16.09.20 12:02, Wei Yang wrote: >> On Wed, Sep 16, 2020 at 09:30:41AM +0200, David Hildenbrand wrote: >>> "mem" in the name already indicates the root, similar to >>> release_mem_region() and devm_request_mem_region(). Make it implicit. >>> The only single caller always passes iomem_resource, other parents are >>> not applicable. >>> >> >> Looks good to me. >> >> Reviewed-by: Wei Yang <richard.weiyang@linux.alibaba.com> >> > >Thanks for the review! > Would you send another version? I didn't take a look into the following patches, since the 4th is missed. >-- >Thanks, > >David / dhildenb
On 16.09.20 14:10, Wei Yang wrote: > On Wed, Sep 16, 2020 at 12:03:20PM +0200, David Hildenbrand wrote: >> On 16.09.20 12:02, Wei Yang wrote: >>> On Wed, Sep 16, 2020 at 09:30:41AM +0200, David Hildenbrand wrote: >>>> "mem" in the name already indicates the root, similar to >>>> release_mem_region() and devm_request_mem_region(). Make it implicit. >>>> The only single caller always passes iomem_resource, other parents are >>>> not applicable. >>>> >>> >>> Looks good to me. >>> >>> Reviewed-by: Wei Yang <richard.weiyang@linux.alibaba.com> >>> >> >> Thanks for the review! >> > > Would you send another version? I didn't take a look into the following > patches, since the 4th is missed. Not planning to send another one as long as there are no further comments. Seems to be an issue on your side because all patches arrived on linux-mm (see https://lore.kernel.org/linux-mm/20200911103459.10306-1-david@redhat.com/) You can find patch #4 at https://lore.kernel.org/linux-mm/20200911103459.10306-5-david@redhat.com/ (which has CC "Wei Yang <richardw.yang@linux.intel.com>")
On Wed, Sep 16, 2020 at 02:16:25PM +0200, David Hildenbrand wrote: >On 16.09.20 14:10, Wei Yang wrote: >> On Wed, Sep 16, 2020 at 12:03:20PM +0200, David Hildenbrand wrote: >>> On 16.09.20 12:02, Wei Yang wrote: >>>> On Wed, Sep 16, 2020 at 09:30:41AM +0200, David Hildenbrand wrote: >>>>> "mem" in the name already indicates the root, similar to >>>>> release_mem_region() and devm_request_mem_region(). Make it implicit. >>>>> The only single caller always passes iomem_resource, other parents are >>>>> not applicable. >>>>> >>>> >>>> Looks good to me. >>>> >>>> Reviewed-by: Wei Yang <richard.weiyang@linux.alibaba.com> >>>> >>> >>> Thanks for the review! >>> >> >> Would you send another version? I didn't take a look into the following >> patches, since the 4th is missed. > >Not planning to send another one as long as there are no further >comments. Seems to be an issue on your side because all patches arrived >on linux-mm (see >https://lore.kernel.org/linux-mm/20200911103459.10306-1-david@redhat.com/) > >You can find patch #4 at >https://lore.kernel.org/linux-mm/20200911103459.10306-5-david@redhat.com/ > >(which has CC "Wei Yang <richardw.yang@linux.intel.com>") > Ok, I managed to apply 4th patch manually... >-- >Thanks, > >David / dhildenb
> "mem" in the name already indicates the root, similar to > release_mem_region() and devm_request_mem_region(). Make it implicit. > The only single caller always passes iomem_resource, other parents are > not applicable. > > Suggested-by: Wei Yang <richard.weiyang@linux.alibaba.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Cc: Kees Cook <keescook@chromium.org> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com> > Cc: Baoquan He <bhe@redhat.com> > Cc: Wei Yang <richard.weiyang@linux.alibaba.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > > Based on next-20200915. Follow up on > "[PATCH v4 0/8] selective merging of system ram resources" [1] > That's in next-20200915. As noted during review of v2 by Wei [2]. > > [1] https://lkml.kernel.org/r/20200911103459.10306-1-david@redhat.com > [2] https://lkml.kernel.org/r/20200915021012.GC2007@L-31X9LVDL-1304.local > > --- > include/linux/ioport.h | 3 +-- > kernel/resource.c | 5 ++--- > mm/memory_hotplug.c | 2 +- > 3 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/include/linux/ioport.h b/include/linux/ioport.h > index 7e61389dcb01..5135d4b86cd6 100644 > --- a/include/linux/ioport.h > +++ b/include/linux/ioport.h > @@ -251,8 +251,7 @@ extern struct resource * __request_region(struct resource *, > extern void __release_region(struct resource *, resource_size_t, > resource_size_t); > #ifdef CONFIG_MEMORY_HOTREMOVE > -extern void release_mem_region_adjustable(struct resource *, resource_size_t, > - resource_size_t); > +extern void release_mem_region_adjustable(resource_size_t, resource_size_t); > #endif > #ifdef CONFIG_MEMORY_HOTPLUG > extern void merge_system_ram_resource(struct resource *res); > diff --git a/kernel/resource.c b/kernel/resource.c > index 7a91b935f4c2..ca2a666e4317 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -1240,7 +1240,6 @@ EXPORT_SYMBOL(__release_region); > #ifdef CONFIG_MEMORY_HOTREMOVE > /** > * release_mem_region_adjustable - release a previously reserved memory region > - * @parent: parent resource descriptor > * @start: resource start address > * @size: resource region size > * > @@ -1258,9 +1257,9 @@ EXPORT_SYMBOL(__release_region); > * assumes that all children remain in the lower address entry for > * simplicity. Enhance this logic when necessary. > */ > -void release_mem_region_adjustable(struct resource *parent, > - resource_size_t start, resource_size_t size) > +void release_mem_region_adjustable(resource_size_t start, resource_size_t size) > { > + struct resource *parent = &iomem_resource; > struct resource *new_res = NULL; > bool alloc_nofail = false; > struct resource **p; > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 553c718226b3..7c5e4744ac51 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -1764,7 +1764,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) > memblock_remove(start, size); > } > > - release_mem_region_adjustable(&iomem_resource, start, size); > + release_mem_region_adjustable(start, size); > > try_offline_node(nid); > Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
diff --git a/include/linux/ioport.h b/include/linux/ioport.h index 7e61389dcb01..5135d4b86cd6 100644 --- a/include/linux/ioport.h +++ b/include/linux/ioport.h @@ -251,8 +251,7 @@ extern struct resource * __request_region(struct resource *, extern void __release_region(struct resource *, resource_size_t, resource_size_t); #ifdef CONFIG_MEMORY_HOTREMOVE -extern void release_mem_region_adjustable(struct resource *, resource_size_t, - resource_size_t); +extern void release_mem_region_adjustable(resource_size_t, resource_size_t); #endif #ifdef CONFIG_MEMORY_HOTPLUG extern void merge_system_ram_resource(struct resource *res); diff --git a/kernel/resource.c b/kernel/resource.c index 7a91b935f4c2..ca2a666e4317 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -1240,7 +1240,6 @@ EXPORT_SYMBOL(__release_region); #ifdef CONFIG_MEMORY_HOTREMOVE /** * release_mem_region_adjustable - release a previously reserved memory region - * @parent: parent resource descriptor * @start: resource start address * @size: resource region size * @@ -1258,9 +1257,9 @@ EXPORT_SYMBOL(__release_region); * assumes that all children remain in the lower address entry for * simplicity. Enhance this logic when necessary. */ -void release_mem_region_adjustable(struct resource *parent, - resource_size_t start, resource_size_t size) +void release_mem_region_adjustable(resource_size_t start, resource_size_t size) { + struct resource *parent = &iomem_resource; struct resource *new_res = NULL; bool alloc_nofail = false; struct resource **p; diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 553c718226b3..7c5e4744ac51 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -1764,7 +1764,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) memblock_remove(start, size); } - release_mem_region_adjustable(&iomem_resource, start, size); + release_mem_region_adjustable(start, size); try_offline_node(nid);
"mem" in the name already indicates the root, similar to release_mem_region() and devm_request_mem_region(). Make it implicit. The only single caller always passes iomem_resource, other parents are not applicable. Suggested-by: Wei Yang <richard.weiyang@linux.alibaba.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Michal Hocko <mhocko@suse.com> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Kees Cook <keescook@chromium.org> Cc: Ard Biesheuvel <ardb@kernel.org> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com> Cc: Baoquan He <bhe@redhat.com> Cc: Wei Yang <richard.weiyang@linux.alibaba.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- Based on next-20200915. Follow up on "[PATCH v4 0/8] selective merging of system ram resources" [1] That's in next-20200915. As noted during review of v2 by Wei [2]. [1] https://lkml.kernel.org/r/20200911103459.10306-1-david@redhat.com [2] https://lkml.kernel.org/r/20200915021012.GC2007@L-31X9LVDL-1304.local --- include/linux/ioport.h | 3 +-- kernel/resource.c | 5 ++--- mm/memory_hotplug.c | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-)