[PATCH 1/2] mm/memory_hotplug: Change pfn_to_section_nr/section_nr_to_pfn macro to inline function

[PATCH 1/2] mm/memory_hotplug: Change pfn_to_section_nr/section_nr_to_pfn macro to inline function

[YASUAKI ISHIMATSU]

pfn_to_section_nr() and section_nr_to_pfn() are defined as macro.
pfn_to_section_nr() has no issue even if it is defined as macro.
But section_nr_to_pfn() has overflow issue if sec is defined as int.

section_nr_to_pfn() just shifts sec by PFN_SECTION_SHIFT. If sec
is defined as unsigned long, section_nr_to_pfn() returns pfn as 64
bit value. But if sec is defined as int, section_nr_to_pfn() returns
pfn as 32 bit value.

__remove_section() calculates start_pfn using section_nr_to_pfn() and
scn_nr defined as int. So if hot-removed memory address is over 16TB,
overflow issue occurs and section_nr_to_pfn() does not calculate
correct pfn.

To make callers use proper arg, the patch changes the macros to
inline functions.

Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
---
 include/linux/mmzone.h | 10 ++++++++--
 mm/memory_hotplug.c    |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index ef6a13b..6ae12b2 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1073,8 +1073,14 @@ static inline unsigned long early_pfn_to_nid(unsigned long pfn)
 #error Allocator MAX_ORDER exceeds SECTION_SIZE
 #endif

-#define pfn_to_section_nr(pfn) ((pfn) >> PFN_SECTION_SHIFT)
-#define section_nr_to_pfn(sec) ((sec) << PFN_SECTION_SHIFT)
+static inline unsigned long pfn_to_section_nr(unsigned long pfn)
+{
+	return pfn >> PFN_SECTION_SHIFT;
+}
+static inline unsigned long section_nr_to_pfn(unsigned long sec)
+{
+	return sec << PFN_SECTION_SHIFT;
+}

 #define SECTION_ALIGN_UP(pfn)	(((pfn) + PAGES_PER_SECTION - 1) & PAGE_SECTION_MASK)
 #define SECTION_ALIGN_DOWN(pfn)	((pfn) & PAGE_SECTION_MASK)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b63d7d1..38c3c37 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -798,7 +798,7 @@ static int __remove_section(struct zone *zone, struct mem_section *ms,
 		return ret;

 	scn_nr = __section_nr(ms);
-	start_pfn = section_nr_to_pfn(scn_nr);
+	start_pfn = section_nr_to_pfn((unsigned long)scn_nr);
 	__remove_zone(zone, start_pfn);

 	sparse_remove_one_section(zone, ms, map_offset);
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] mm/memory_hotplug: define find_{smallest|biggest}_section_pfn as unsigned long

[YASUAKI ISHIMATSU]

find_{smallest|biggest}_section_pfn()s find the smallest/biggest section
and return the pfn of the section. But the functions are defined as int.
So the functions always return 0x00000000 - 0xffffffff. It means
if memory address is over 16TB, the functions does not work correctly.

To handle 64 bit value, the patch defines find_{smallest|biggest}_section_pfn()
as unsigned long.

Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
---
 mm/memory_hotplug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 38c3c37..120e45b 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -582,7 +582,7 @@ int __ref __add_pages(int nid, struct zone *zone, unsigned long phys_start_pfn,

 #ifdef CONFIG_MEMORY_HOTREMOVE
 /* find the smallest valid pfn in the range [start_pfn, end_pfn) */
-static int find_smallest_section_pfn(int nid, struct zone *zone,
+static unsigned long find_smallest_section_pfn(int nid, struct zone *zone,
 				     unsigned long start_pfn,
 				     unsigned long end_pfn)
 {
@@ -607,7 +607,7 @@ static int find_smallest_section_pfn(int nid, struct zone *zone,
 }

 /* find the biggest valid pfn in the range [start_pfn, end_pfn). */
-static int find_biggest_section_pfn(int nid, struct zone *zone,
+static unsigned long find_biggest_section_pfn(int nid, struct zone *zone,
 				    unsigned long start_pfn,
 				    unsigned long end_pfn)
 {
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] mm/memory_hotplug: define find_{smallest|biggest}_section_pfn as unsigned long

[Michal Hocko]

On Fri 15-09-17 22:53:49, YASUAKI ISHIMATSU wrote:
> find_{smallest|biggest}_section_pfn()s find the smallest/biggest section
> and return the pfn of the section. But the functions are defined as int.
> So the functions always return 0x00000000 - 0xffffffff. It means
> if memory address is over 16TB, the functions does not work correctly.
> 
> To handle 64 bit value, the patch defines find_{smallest|biggest}_section_pfn()
> as unsigned long.
> 

Fixes: 815121d2b5cd ("memory_hotplug: clear zone when removing the memory")
> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/memory_hotplug.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 38c3c37..120e45b 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -582,7 +582,7 @@ int __ref __add_pages(int nid, struct zone *zone, unsigned long phys_start_pfn,
> 
>  #ifdef CONFIG_MEMORY_HOTREMOVE
>  /* find the smallest valid pfn in the range [start_pfn, end_pfn) */
> -static int find_smallest_section_pfn(int nid, struct zone *zone,
> +static unsigned long find_smallest_section_pfn(int nid, struct zone *zone,
>  				     unsigned long start_pfn,
>  				     unsigned long end_pfn)
>  {
> @@ -607,7 +607,7 @@ static int find_smallest_section_pfn(int nid, struct zone *zone,
>  }
> 
>  /* find the biggest valid pfn in the range [start_pfn, end_pfn). */
> -static int find_biggest_section_pfn(int nid, struct zone *zone,
> +static unsigned long find_biggest_section_pfn(int nid, struct zone *zone,
>  				    unsigned long start_pfn,
>  				    unsigned long end_pfn)
>  {
> -- 
> 1.8.3.1

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] mm/memory_hotplug: Change pfn_to_section_nr/section_nr_to_pfn macro to inline function

[Michal Hocko]

On Fri 15-09-17 22:52:20, YASUAKI ISHIMATSU wrote:
> pfn_to_section_nr() and section_nr_to_pfn() are defined as macro.
> pfn_to_section_nr() has no issue even if it is defined as macro.
> But section_nr_to_pfn() has overflow issue if sec is defined as int.
> 
> section_nr_to_pfn() just shifts sec by PFN_SECTION_SHIFT. If sec
> is defined as unsigned long, section_nr_to_pfn() returns pfn as 64
> bit value. But if sec is defined as int, section_nr_to_pfn() returns
> pfn as 32 bit value.
> 
> __remove_section() calculates start_pfn using section_nr_to_pfn() and
> scn_nr defined as int. So if hot-removed memory address is over 16TB,
> overflow issue occurs and section_nr_to_pfn() does not calculate
> correct pfn.
> 
> To make callers use proper arg, the patch changes the macros to
> inline functions.
> 

I guess the following is due

Fixes: 815121d2b5cd ("memory_hotplug: clear zone when removing the memory")
> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!
> ---
>  include/linux/mmzone.h | 10 ++++++++--
>  mm/memory_hotplug.c    |  2 +-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index ef6a13b..6ae12b2 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1073,8 +1073,14 @@ static inline unsigned long early_pfn_to_nid(unsigned long pfn)
>  #error Allocator MAX_ORDER exceeds SECTION_SIZE
>  #endif
> 
> -#define pfn_to_section_nr(pfn) ((pfn) >> PFN_SECTION_SHIFT)
> -#define section_nr_to_pfn(sec) ((sec) << PFN_SECTION_SHIFT)
> +static inline unsigned long pfn_to_section_nr(unsigned long pfn)
> +{
> +	return pfn >> PFN_SECTION_SHIFT;
> +}
> +static inline unsigned long section_nr_to_pfn(unsigned long sec)
> +{
> +	return sec << PFN_SECTION_SHIFT;
> +}
> 
>  #define SECTION_ALIGN_UP(pfn)	(((pfn) + PAGES_PER_SECTION - 1) & PAGE_SECTION_MASK)
>  #define SECTION_ALIGN_DOWN(pfn)	((pfn) & PAGE_SECTION_MASK)
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index b63d7d1..38c3c37 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -798,7 +798,7 @@ static int __remove_section(struct zone *zone, struct mem_section *ms,
>  		return ret;
> 
>  	scn_nr = __section_nr(ms);
> -	start_pfn = section_nr_to_pfn(scn_nr);
> +	start_pfn = section_nr_to_pfn((unsigned long)scn_nr);
>  	__remove_zone(zone, start_pfn);
> 
>  	sparse_remove_one_section(zone, ms, map_offset);
> -- 
> 1.8.3.1

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>