linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mm: sparse: remove __section_nr() function
       [not found] <CGME20210702094457epcas1p295611b5799befffd016b8fccf3adceff@epcas1p2.samsung.com>
@ 2021-07-02  9:41 ` Ohhoon Kwon
       [not found]   ` <CGME20210702094457epcas1p3ddac76bd3cc3e5b93fadb897cdb6dfd0@epcas1p3.samsung.com>
                     ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Ohhoon Kwon @ 2021-07-02  9:41 UTC (permalink / raw)
  To: david, ohoono.kwon, akpm, mhocko
  Cc: bhe, rppt, ohkwon1043, linux-mm, linux-kernel

This series contains cleanups to remove __section_nr().

When CONFIG_SPARSEMEM_EXTREME is enabled, __section_nr() could be
costly since it iterates all section roots to check if the given
mem_section is in its range.

On the other hand, __nr_to_section which converts section_nr to
mem_section can be done in O(1).

The only users of __section_nr() was section_mark_present() and
find_memory_block().

PATCH 1 & 2 changes both functions to use section_nr instead of
mem_section.
PATCH 3 finally removes __section_nr() function.

More details can be found in each changelogs.

Ohhoon Kwon (3):
  mm: sparse: pass section_nr to section_mark_present
  mm: sparse: pass section_nr to find_memory_block
  mm: sparse: remove __section_nr() function

 .../platforms/pseries/hotplug-memory.c        |  4 +--
 drivers/base/memory.c                         |  4 +--
 include/linux/memory.h                        |  2 +-
 include/linux/mmzone.h                        |  1 -
 mm/sparse.c                                   | 35 +++----------------
 5 files changed, 9 insertions(+), 37 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] mm: sparse: pass section_nr to section_mark_present
       [not found]   ` <CGME20210702094457epcas1p3ddac76bd3cc3e5b93fadb897cdb6dfd0@epcas1p3.samsung.com>
@ 2021-07-02  9:41     ` Ohhoon Kwon
  2021-07-02 12:13       ` Michal Hocko
  2021-07-02 17:54       ` David Hildenbrand
  0 siblings, 2 replies; 11+ messages in thread
From: Ohhoon Kwon @ 2021-07-02  9:41 UTC (permalink / raw)
  To: david, ohoono.kwon, akpm, mhocko
  Cc: bhe, rppt, ohkwon1043, linux-mm, linux-kernel

With CONFIG_SPARSEMEM_EXTREME enabled, __section_nr() which converts
mem_section to section_nr could be costly since it iterates all
section roots to check if the given mem_section is in its range.

On the other hand, __nr_to_section() which converts section_nr to
mem_section can be done in O(1).

Let's pass section_nr instead of mem_section ptr to
section_mark_present() in order to reduce needless iterations.

Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>
---
 mm/sparse.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mm/sparse.c b/mm/sparse.c
index 55c18aff3e42..4a2700e9a65f 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -186,13 +186,14 @@ void __meminit mminit_validate_memmodel_limits(unsigned long *start_pfn,
  * those loops early.
  */
 unsigned long __highest_present_section_nr;
-static void section_mark_present(struct mem_section *ms)
+static void section_mark_present(unsigned long section_nr)
 {
-	unsigned long section_nr = __section_nr(ms);
+	struct mem_section *ms;
 
 	if (section_nr > __highest_present_section_nr)
 		__highest_present_section_nr = section_nr;
 
+	ms = __nr_to_section(section_nr);
 	ms->section_mem_map |= SECTION_MARKED_PRESENT;
 }
 
@@ -279,7 +280,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
 		if (!ms->section_mem_map) {
 			ms->section_mem_map = sparse_encode_early_nid(nid) |
 							SECTION_IS_ONLINE;
-			section_mark_present(ms);
+			section_mark_present(section);
 		}
 	}
 }
@@ -933,7 +934,7 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
 
 	ms = __nr_to_section(section_nr);
 	set_section_nid(section_nr, nid);
-	section_mark_present(ms);
+	section_mark_present(section_nr);
 
 	/* Align memmap to section boundary in the subsection case */
 	if (section_nr_to_pfn(section_nr) != start_pfn)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] mm: sparse: pass section_nr to find_memory_block
       [not found]   ` <CGME20210702094457epcas1p40fba85e22861cf1cc85a085719030c24@epcas1p4.samsung.com>
@ 2021-07-02  9:41     ` Ohhoon Kwon
  2021-07-02 12:17       ` Michal Hocko
  2021-07-02 17:55       ` David Hildenbrand
  0 siblings, 2 replies; 11+ messages in thread
From: Ohhoon Kwon @ 2021-07-02  9:41 UTC (permalink / raw)
  To: david, ohoono.kwon, akpm, mhocko
  Cc: bhe, rppt, ohkwon1043, linux-mm, linux-kernel

With CONFIG_SPARSEMEM_EXTREME enabled, __section_nr() which converts
mem_section to section_nr could be costly since it iterates all
section roots to check if the given mem_section is in its range.

On the other hand, __nr_to_section() which converts section_nr to
mem_section can be done in O(1).

Let's pass section_nr instead of mem_section ptr to
find_memory_block() in order to reduce needless iterations.

Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>
---
 arch/powerpc/platforms/pseries/hotplug-memory.c | 4 +---
 drivers/base/memory.c                           | 4 ++--
 include/linux/memory.h                          | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 8377f1f7c78e..905790092e0e 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -211,13 +211,11 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
 static struct memory_block *lmb_to_memblock(struct drmem_lmb *lmb)
 {
 	unsigned long section_nr;
-	struct mem_section *mem_sect;
 	struct memory_block *mem_block;
 
 	section_nr = pfn_to_section_nr(PFN_DOWN(lmb->base_addr));
-	mem_sect = __nr_to_section(section_nr);
 
-	mem_block = find_memory_block(mem_sect);
+	mem_block = find_memory_block(section_nr);
 	return mem_block;
 }
 
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index d5ffaab3cb61..e31598955cc4 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -578,9 +578,9 @@ static struct memory_block *find_memory_block_by_id(unsigned long block_id)
 /*
  * Called under device_hotplug_lock.
  */
-struct memory_block *find_memory_block(struct mem_section *section)
+struct memory_block *find_memory_block(unsigned long section_nr)
 {
-	unsigned long block_id = memory_block_id(__section_nr(section));
+	unsigned long block_id = memory_block_id(section_nr);
 
 	return find_memory_block_by_id(block_id);
 }
diff --git a/include/linux/memory.h b/include/linux/memory.h
index 97e92e8b556a..d9a0b61cd432 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -90,7 +90,7 @@ int create_memory_block_devices(unsigned long start, unsigned long size,
 void remove_memory_block_devices(unsigned long start, unsigned long size);
 extern void memory_dev_init(void);
 extern int memory_notify(unsigned long val, void *v);
-extern struct memory_block *find_memory_block(struct mem_section *);
+extern struct memory_block *find_memory_block(unsigned long section_nr);
 typedef int (*walk_memory_blocks_func_t)(struct memory_block *, void *);
 extern int walk_memory_blocks(unsigned long start, unsigned long size,
 			      void *arg, walk_memory_blocks_func_t func);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] mm: sparse: remove __section_nr() function
       [not found]   ` <CGME20210702094457epcas1p4e181c7b0a18338403a7ffb57f44807fe@epcas1p4.samsung.com>
@ 2021-07-02  9:41     ` Ohhoon Kwon
  2021-07-02 12:18       ` Michal Hocko
  0 siblings, 1 reply; 11+ messages in thread
From: Ohhoon Kwon @ 2021-07-02  9:41 UTC (permalink / raw)
  To: david, ohoono.kwon, akpm, mhocko
  Cc: bhe, rppt, ohkwon1043, linux-mm, linux-kernel

__section_nr() was used to convert struct mem_section * to section_nr.

With CONFIG_SPARSEMEM_EXTREME enabled, however, __section_nr() can be
costly since it iterates all section roots to check if the given
mem_section is in its range.

On the other hand, __nr_to_section() which converts section_nr to
mem_section can be done in O(1).

The only users of __section_nr() was section_mark_present() and
find_memory_block().

Since I changed both functions to use section_nr directly in the
preceeding patches, let's remove __section_nr() which has no users.

Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>
---
 include/linux/mmzone.h |  1 -
 mm/sparse.c            | 26 --------------------------
 2 files changed, 27 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 0d53eba1c383..8931f95cf885 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1321,7 +1321,6 @@ static inline struct mem_section *__nr_to_section(unsigned long nr)
 		return NULL;
 	return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK];
 }
-extern unsigned long __section_nr(struct mem_section *ms);
 extern size_t mem_section_usage_size(void);
 
 /*
diff --git a/mm/sparse.c b/mm/sparse.c
index 4a2700e9a65f..1b32d15593e4 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -108,32 +108,6 @@ static inline int sparse_index_init(unsigned long section_nr, int nid)
 }
 #endif
 
-#ifdef CONFIG_SPARSEMEM_EXTREME
-unsigned long __section_nr(struct mem_section *ms)
-{
-	unsigned long root_nr;
-	struct mem_section *root = NULL;
-
-	for (root_nr = 0; root_nr < NR_SECTION_ROOTS; root_nr++) {
-		root = __nr_to_section(root_nr * SECTIONS_PER_ROOT);
-		if (!root)
-			continue;
-
-		if ((ms >= root) && (ms < (root + SECTIONS_PER_ROOT)))
-		     break;
-	}
-
-	VM_BUG_ON(!root);
-
-	return (root_nr * SECTIONS_PER_ROOT) + (ms - root);
-}
-#else
-unsigned long __section_nr(struct mem_section *ms)
-{
-	return (unsigned long)(ms - mem_section[0]);
-}
-#endif
-
 /*
  * During early boot, before section_mem_map is used for an actual
  * mem_map, we use section_mem_map to store the section's NUMA
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] mm: sparse: pass section_nr to section_mark_present
  2021-07-02  9:41     ` [PATCH 1/3] mm: sparse: pass section_nr to section_mark_present Ohhoon Kwon
@ 2021-07-02 12:13       ` Michal Hocko
  2021-07-02 17:54       ` David Hildenbrand
  1 sibling, 0 replies; 11+ messages in thread
From: Michal Hocko @ 2021-07-02 12:13 UTC (permalink / raw)
  To: Ohhoon Kwon; +Cc: david, akpm, bhe, rppt, ohkwon1043, linux-mm, linux-kernel

On Fri 02-07-21 18:41:30, Ohhoon Kwon wrote:
> With CONFIG_SPARSEMEM_EXTREME enabled, __section_nr() which converts
> mem_section to section_nr could be costly since it iterates all
> section roots to check if the given mem_section is in its range.
> 
> On the other hand, __nr_to_section() which converts section_nr to
> mem_section can be done in O(1).
> 
> Let's pass section_nr instead of mem_section ptr to
> section_mark_present() in order to reduce needless iterations.

It is indeed wasteful to spend time on something that is already known.
Both callers have already determined both the section number and the
section so why not just pass both to section_mark_present?
One could argue that from an API point of view it is a bad practice to
have two indipendent arguments referring to the same underlying object,
and I would agree, but this is not really a something that has a wider
use so it is more of a helper. Maybe we want to make that more explicit
via __ prefix.
 
> Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>
> ---
>  mm/sparse.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 55c18aff3e42..4a2700e9a65f 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -186,13 +186,14 @@ void __meminit mminit_validate_memmodel_limits(unsigned long *start_pfn,
>   * those loops early.
>   */
>  unsigned long __highest_present_section_nr;
> -static void section_mark_present(struct mem_section *ms)
> +static void section_mark_present(unsigned long section_nr)
>  {
> -	unsigned long section_nr = __section_nr(ms);
> +	struct mem_section *ms;
>  
>  	if (section_nr > __highest_present_section_nr)
>  		__highest_present_section_nr = section_nr;
>  
> +	ms = __nr_to_section(section_nr);
>  	ms->section_mem_map |= SECTION_MARKED_PRESENT;
>  }
>  
> @@ -279,7 +280,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
>  		if (!ms->section_mem_map) {
>  			ms->section_mem_map = sparse_encode_early_nid(nid) |
>  							SECTION_IS_ONLINE;
> -			section_mark_present(ms);
> +			section_mark_present(section);
>  		}
>  	}
>  }
> @@ -933,7 +934,7 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
>  
>  	ms = __nr_to_section(section_nr);
>  	set_section_nid(section_nr, nid);
> -	section_mark_present(ms);
> +	section_mark_present(section_nr);
>  
>  	/* Align memmap to section boundary in the subsection case */
>  	if (section_nr_to_pfn(section_nr) != start_pfn)
> -- 
> 2.17.1

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mm: sparse: pass section_nr to find_memory_block
  2021-07-02  9:41     ` [PATCH 2/3] mm: sparse: pass section_nr to find_memory_block Ohhoon Kwon
@ 2021-07-02 12:17       ` Michal Hocko
  2021-07-02 17:55       ` David Hildenbrand
  1 sibling, 0 replies; 11+ messages in thread
From: Michal Hocko @ 2021-07-02 12:17 UTC (permalink / raw)
  To: Ohhoon Kwon; +Cc: david, akpm, bhe, rppt, ohkwon1043, linux-mm, linux-kernel

On Fri 02-07-21 18:41:31, Ohhoon Kwon wrote:
> With CONFIG_SPARSEMEM_EXTREME enabled, __section_nr() which converts
> mem_section to section_nr could be costly since it iterates all
> section roots to check if the given mem_section is in its range.
> 
> On the other hand, __nr_to_section() which converts section_nr to
> mem_section can be done in O(1).
> 
> Let's pass section_nr instead of mem_section ptr to
> find_memory_block() in order to reduce needless iterations.

Yeah, it seems like the only existing user (maybe there used to be more
in the past - haven't checked) is just doing pointless translation to
comply with the API that doesn't fit it. Just to undo all that work
in find_memory_block 

Nice cleanup and potentially even a performance improvement!
 
> Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>

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

> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c | 4 +---
>  drivers/base/memory.c                           | 4 ++--
>  include/linux/memory.h                          | 2 +-
>  3 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 8377f1f7c78e..905790092e0e 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -211,13 +211,11 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
>  static struct memory_block *lmb_to_memblock(struct drmem_lmb *lmb)
>  {
>  	unsigned long section_nr;
> -	struct mem_section *mem_sect;
>  	struct memory_block *mem_block;
>  
>  	section_nr = pfn_to_section_nr(PFN_DOWN(lmb->base_addr));
> -	mem_sect = __nr_to_section(section_nr);
>  
> -	mem_block = find_memory_block(mem_sect);
> +	mem_block = find_memory_block(section_nr);
>  	return mem_block;
>  }
>  
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index d5ffaab3cb61..e31598955cc4 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -578,9 +578,9 @@ static struct memory_block *find_memory_block_by_id(unsigned long block_id)
>  /*
>   * Called under device_hotplug_lock.
>   */
> -struct memory_block *find_memory_block(struct mem_section *section)
> +struct memory_block *find_memory_block(unsigned long section_nr)
>  {
> -	unsigned long block_id = memory_block_id(__section_nr(section));
> +	unsigned long block_id = memory_block_id(section_nr);
>  
>  	return find_memory_block_by_id(block_id);
>  }
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index 97e92e8b556a..d9a0b61cd432 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -90,7 +90,7 @@ int create_memory_block_devices(unsigned long start, unsigned long size,
>  void remove_memory_block_devices(unsigned long start, unsigned long size);
>  extern void memory_dev_init(void);
>  extern int memory_notify(unsigned long val, void *v);
> -extern struct memory_block *find_memory_block(struct mem_section *);
> +extern struct memory_block *find_memory_block(unsigned long section_nr);
>  typedef int (*walk_memory_blocks_func_t)(struct memory_block *, void *);
>  extern int walk_memory_blocks(unsigned long start, unsigned long size,
>  			      void *arg, walk_memory_blocks_func_t func);
> -- 
> 2.17.1

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] mm: sparse: remove __section_nr() function
  2021-07-02  9:41     ` [PATCH 3/3] mm: sparse: remove __section_nr() function Ohhoon Kwon
@ 2021-07-02 12:18       ` Michal Hocko
  2021-07-02 17:57         ` David Hildenbrand
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Hocko @ 2021-07-02 12:18 UTC (permalink / raw)
  To: Ohhoon Kwon; +Cc: david, akpm, bhe, rppt, ohkwon1043, linux-mm, linux-kernel

On Fri 02-07-21 18:41:32, Ohhoon Kwon wrote:
> __section_nr() was used to convert struct mem_section * to section_nr.
> 
> With CONFIG_SPARSEMEM_EXTREME enabled, however, __section_nr() can be
> costly since it iterates all section roots to check if the given
> mem_section is in its range.
> 
> On the other hand, __nr_to_section() which converts section_nr to
> mem_section can be done in O(1).
> 
> The only users of __section_nr() was section_mark_present() and
> find_memory_block().
> 
> Since I changed both functions to use section_nr directly in the
> preceeding patches, let's remove __section_nr() which has no users.
> 
> Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>

I would go with a much shorter changelog. The function is not used
anymore so it can be simply dropped.
Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  include/linux/mmzone.h |  1 -
>  mm/sparse.c            | 26 --------------------------
>  2 files changed, 27 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 0d53eba1c383..8931f95cf885 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1321,7 +1321,6 @@ static inline struct mem_section *__nr_to_section(unsigned long nr)
>  		return NULL;
>  	return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK];
>  }
> -extern unsigned long __section_nr(struct mem_section *ms);
>  extern size_t mem_section_usage_size(void);
>  
>  /*
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 4a2700e9a65f..1b32d15593e4 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -108,32 +108,6 @@ static inline int sparse_index_init(unsigned long section_nr, int nid)
>  }
>  #endif
>  
> -#ifdef CONFIG_SPARSEMEM_EXTREME
> -unsigned long __section_nr(struct mem_section *ms)
> -{
> -	unsigned long root_nr;
> -	struct mem_section *root = NULL;
> -
> -	for (root_nr = 0; root_nr < NR_SECTION_ROOTS; root_nr++) {
> -		root = __nr_to_section(root_nr * SECTIONS_PER_ROOT);
> -		if (!root)
> -			continue;
> -
> -		if ((ms >= root) && (ms < (root + SECTIONS_PER_ROOT)))
> -		     break;
> -	}
> -
> -	VM_BUG_ON(!root);
> -
> -	return (root_nr * SECTIONS_PER_ROOT) + (ms - root);
> -}
> -#else
> -unsigned long __section_nr(struct mem_section *ms)
> -{
> -	return (unsigned long)(ms - mem_section[0]);
> -}
> -#endif
> -
>  /*
>   * During early boot, before section_mem_map is used for an actual
>   * mem_map, we use section_mem_map to store the section's NUMA
> -- 
> 2.17.1

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] mm: sparse: remove __section_nr() function
  2021-07-02  9:41 ` [PATCH 0/3] mm: sparse: remove __section_nr() function Ohhoon Kwon
                     ` (2 preceding siblings ...)
       [not found]   ` <CGME20210702094457epcas1p4e181c7b0a18338403a7ffb57f44807fe@epcas1p4.samsung.com>
@ 2021-07-02 12:58   ` Mike Rapoport
  3 siblings, 0 replies; 11+ messages in thread
From: Mike Rapoport @ 2021-07-02 12:58 UTC (permalink / raw)
  To: Ohhoon Kwon; +Cc: david, akpm, mhocko, bhe, ohkwon1043, linux-mm, linux-kernel

On Fri, Jul 02, 2021 at 06:41:29PM +0900, Ohhoon Kwon wrote:
> This series contains cleanups to remove __section_nr().
> 
> When CONFIG_SPARSEMEM_EXTREME is enabled, __section_nr() could be
> costly since it iterates all section roots to check if the given
> mem_section is in its range.
> 
> On the other hand, __nr_to_section which converts section_nr to
> mem_section can be done in O(1).
> 
> The only users of __section_nr() was section_mark_present() and
> find_memory_block().
> 
> PATCH 1 & 2 changes both functions to use section_nr instead of
> mem_section.
> PATCH 3 finally removes __section_nr() function.
> 
> More details can be found in each changelogs.
> 
> Ohhoon Kwon (3):
>   mm: sparse: pass section_nr to section_mark_present
>   mm: sparse: pass section_nr to find_memory_block
>   mm: sparse: remove __section_nr() function
> 
>  .../platforms/pseries/hotplug-memory.c        |  4 +--
>  drivers/base/memory.c                         |  4 +--
>  include/linux/memory.h                        |  2 +-
>  include/linux/mmzone.h                        |  1 -
>  mm/sparse.c                                   | 35 +++----------------
>  5 files changed, 9 insertions(+), 37 deletions(-)
> 
> -- 
> 2.17.1
 
For the series:

Acked-by: Mike Rapoport <rppt@linux.ibm.com>

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] mm: sparse: pass section_nr to section_mark_present
  2021-07-02  9:41     ` [PATCH 1/3] mm: sparse: pass section_nr to section_mark_present Ohhoon Kwon
  2021-07-02 12:13       ` Michal Hocko
@ 2021-07-02 17:54       ` David Hildenbrand
  1 sibling, 0 replies; 11+ messages in thread
From: David Hildenbrand @ 2021-07-02 17:54 UTC (permalink / raw)
  To: Ohhoon Kwon, akpm, mhocko; +Cc: bhe, rppt, ohkwon1043, linux-mm, linux-kernel

On 02.07.21 11:41, Ohhoon Kwon wrote:
> With CONFIG_SPARSEMEM_EXTREME enabled, __section_nr() which converts
> mem_section to section_nr could be costly since it iterates all
> section roots to check if the given mem_section is in its range.
> 
> On the other hand, __nr_to_section() which converts section_nr to
> mem_section can be done in O(1).
> 
> Let's pass section_nr instead of mem_section ptr to
> section_mark_present() in order to reduce needless iterations.
> 
> Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>
> ---
>   mm/sparse.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 55c18aff3e42..4a2700e9a65f 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -186,13 +186,14 @@ void __meminit mminit_validate_memmodel_limits(unsigned long *start_pfn,
>    * those loops early.
>    */
>   unsigned long __highest_present_section_nr;
> -static void section_mark_present(struct mem_section *ms)
> +static void section_mark_present(unsigned long section_nr)
>   {
> -	unsigned long section_nr = __section_nr(ms);
> +	struct mem_section *ms;
>   
>   	if (section_nr > __highest_present_section_nr)
>   		__highest_present_section_nr = section_nr;
>   
> +	ms = __nr_to_section(section_nr);
>   	ms->section_mem_map |= SECTION_MARKED_PRESENT;
>   }
>   
> @@ -279,7 +280,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
>   		if (!ms->section_mem_map) {
>   			ms->section_mem_map = sparse_encode_early_nid(nid) |
>   							SECTION_IS_ONLINE;
> -			section_mark_present(ms);
> +			section_mark_present(section);
>   		}
>   	}
>   }
> @@ -933,7 +934,7 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
>   
>   	ms = __nr_to_section(section_nr);
>   	set_section_nid(section_nr, nid);
> -	section_mark_present(ms);
> +	section_mark_present(section_nr);
>   
>   	/* Align memmap to section boundary in the subsection case */
>   	if (section_nr_to_pfn(section_nr) != start_pfn)
> 

Fine for me either like this, or as Michal suggested. It's an internal 
helper either way.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mm: sparse: pass section_nr to find_memory_block
  2021-07-02  9:41     ` [PATCH 2/3] mm: sparse: pass section_nr to find_memory_block Ohhoon Kwon
  2021-07-02 12:17       ` Michal Hocko
@ 2021-07-02 17:55       ` David Hildenbrand
  1 sibling, 0 replies; 11+ messages in thread
From: David Hildenbrand @ 2021-07-02 17:55 UTC (permalink / raw)
  To: Ohhoon Kwon, akpm, mhocko; +Cc: bhe, rppt, ohkwon1043, linux-mm, linux-kernel

On 02.07.21 11:41, Ohhoon Kwon wrote:
> With CONFIG_SPARSEMEM_EXTREME enabled, __section_nr() which converts
> mem_section to section_nr could be costly since it iterates all
> section roots to check if the given mem_section is in its range.
> 
> On the other hand, __nr_to_section() which converts section_nr to
> mem_section can be done in O(1).
> 
> Let's pass section_nr instead of mem_section ptr to
> find_memory_block() in order to reduce needless iterations.
> 
> Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>


Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] mm: sparse: remove __section_nr() function
  2021-07-02 12:18       ` Michal Hocko
@ 2021-07-02 17:57         ` David Hildenbrand
  0 siblings, 0 replies; 11+ messages in thread
From: David Hildenbrand @ 2021-07-02 17:57 UTC (permalink / raw)
  To: Michal Hocko, Ohhoon Kwon
  Cc: akpm, bhe, rppt, ohkwon1043, linux-mm, linux-kernel

On 02.07.21 14:18, Michal Hocko wrote:
> On Fri 02-07-21 18:41:32, Ohhoon Kwon wrote:
>> __section_nr() was used to convert struct mem_section * to section_nr.
>>
>> With CONFIG_SPARSEMEM_EXTREME enabled, however, __section_nr() can be
>> costly since it iterates all section roots to check if the given
>> mem_section is in its range.
>>
>> On the other hand, __nr_to_section() which converts section_nr to
>> mem_section can be done in O(1).
>>
>> The only users of __section_nr() was section_mark_present() and
>> find_memory_block().
>>
>> Since I changed both functions to use section_nr directly in the
>> preceeding patches, let's remove __section_nr() which has no users.
>>
>> Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>
> 
> I would go with a much shorter changelog. The function is not used
> anymore so it can be simply dropped.
> Acked-by: Michal Hocko <mhocko@suse.com>

Agreed, also avoid the use of "I" in patches.

Use something like

"As the last users of __section_nr() are gone, let's remove now unused 
__section_nr()."

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-07-02 17:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210702094457epcas1p295611b5799befffd016b8fccf3adceff@epcas1p2.samsung.com>
2021-07-02  9:41 ` [PATCH 0/3] mm: sparse: remove __section_nr() function Ohhoon Kwon
     [not found]   ` <CGME20210702094457epcas1p3ddac76bd3cc3e5b93fadb897cdb6dfd0@epcas1p3.samsung.com>
2021-07-02  9:41     ` [PATCH 1/3] mm: sparse: pass section_nr to section_mark_present Ohhoon Kwon
2021-07-02 12:13       ` Michal Hocko
2021-07-02 17:54       ` David Hildenbrand
     [not found]   ` <CGME20210702094457epcas1p40fba85e22861cf1cc85a085719030c24@epcas1p4.samsung.com>
2021-07-02  9:41     ` [PATCH 2/3] mm: sparse: pass section_nr to find_memory_block Ohhoon Kwon
2021-07-02 12:17       ` Michal Hocko
2021-07-02 17:55       ` David Hildenbrand
     [not found]   ` <CGME20210702094457epcas1p4e181c7b0a18338403a7ffb57f44807fe@epcas1p4.samsung.com>
2021-07-02  9:41     ` [PATCH 3/3] mm: sparse: remove __section_nr() function Ohhoon Kwon
2021-07-02 12:18       ` Michal Hocko
2021-07-02 17:57         ` David Hildenbrand
2021-07-02 12:58   ` [PATCH 0/3] " Mike Rapoport

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).