"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 <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Pankaj Gupta <[email protected]>
Cc: Baoquan He <[email protected]>
Cc: Wei Yang <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
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/[email protected]
[2] https://lkml.kernel.org/r/[email protected]
---
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 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 <[email protected]>
>Suggested-by: Wei Yang <[email protected]>
>Cc: Andrew Morton <[email protected]>
>Cc: Michal Hocko <[email protected]>
>Cc: Dan Williams <[email protected]>
>Cc: Jason Gunthorpe <[email protected]>
>Cc: Kees Cook <[email protected]>
>Cc: Ard Biesheuvel <[email protected]>
>Cc: Pankaj Gupta <[email protected]>
>Cc: Baoquan He <[email protected]>
>Cc: Wei Yang <[email protected]>
>Signed-off-by: David Hildenbrand <[email protected]>
>---
>
>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/[email protected]
>[2] https://lkml.kernel.org/r/[email protected]
>
>---
> 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
--
Wei Yang
Help you, Help me
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 <[email protected]>
>
Thanks for the review!
--
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 <[email protected]>
>>>
>>
>> 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/[email protected]/)
You can find patch #4 at
https://lore.kernel.org/linux-mm/[email protected]/
(which has CC "Wei Yang <[email protected]>")
--
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 <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Pankaj Gupta <[email protected]>
> Cc: Baoquan He <[email protected]>
> Cc: Wei Yang <[email protected]>
> Signed-off-by: David Hildenbrand <[email protected]>
> ---
>
> 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/[email protected]
> [2] https://lkml.kernel.org/r/[email protected]
>
> ---
> 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 <[email protected]>