All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] memblock: minor cleanups
@ 2019-02-06 12:10 Mike Rapoport
  2019-02-06 12:10 ` [PATCH 1/2] memblock: remove memblock_{set,clear}_region_flags Mike Rapoport
  2019-02-06 12:10 ` [PATCH 2/2] memblock: split checks whether a region should be skipped to a helper function Mike Rapoport
  0 siblings, 2 replies; 6+ messages in thread
From: Mike Rapoport @ 2019-02-06 12:10 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Michal Hocko, linux-mm, linux-kernel, Mike Rapoport

Hi,

These patches perform some minor cleanups in memblock.
Generated vs mmotm-2019-02-04-17-47.

Mike Rapoport (2):
  memblock: remove memblock_{set,clear}_region_flags
  memblock: split checks whether a region should be skipped to a helper
    function

 include/linux/memblock.h | 12 ----------
 mm/memblock.c            | 62 ++++++++++++++++++++++++------------------------
 2 files changed, 31 insertions(+), 43 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] memblock: remove memblock_{set,clear}_region_flags
  2019-02-06 12:10 [PATCH 0/2] memblock: minor cleanups Mike Rapoport
@ 2019-02-06 12:10 ` Mike Rapoport
  2019-02-07 16:32     ` Souptick Joarder
  2019-02-06 12:10 ` [PATCH 2/2] memblock: split checks whether a region should be skipped to a helper function Mike Rapoport
  1 sibling, 1 reply; 6+ messages in thread
From: Mike Rapoport @ 2019-02-06 12:10 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Michal Hocko, linux-mm, linux-kernel, Mike Rapoport

The memblock API provides dedicated helpers to set or clear a flag on a
memory region, e.g. memblock_{mark,clear}_hotplug().

The memblock_{set,clear}_region_flags() functions are used only by the
memblock internal function that adjusts the region flags.
Drop these functions and use open-coded implementation instead.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 include/linux/memblock.h | 12 ------------
 mm/memblock.c            |  9 ++++++---
 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 71c9e32..32a9a6b 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -317,18 +317,6 @@ void __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
 	for_each_mem_range_rev(i, &memblock.memory, &memblock.reserved,	\
 			       nid, flags, p_start, p_end, p_nid)
 
-static inline void memblock_set_region_flags(struct memblock_region *r,
-					     enum memblock_flags flags)
-{
-	r->flags |= flags;
-}
-
-static inline void memblock_clear_region_flags(struct memblock_region *r,
-					       enum memblock_flags flags)
-{
-	r->flags &= ~flags;
-}
-
 #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
 int memblock_set_node(phys_addr_t base, phys_addr_t size,
 		      struct memblock_type *type, int nid);
diff --git a/mm/memblock.c b/mm/memblock.c
index 0151a5b..af5fe8e 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -851,11 +851,14 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base,
 	if (ret)
 		return ret;
 
-	for (i = start_rgn; i < end_rgn; i++)
+	for (i = start_rgn; i < end_rgn; i++) {
+		struct memblock_region *r = &type->regions[i];
+
 		if (set)
-			memblock_set_region_flags(&type->regions[i], flag);
+			r->flags |= flag;
 		else
-			memblock_clear_region_flags(&type->regions[i], flag);
+			r->flags &= ~flag;
+	}
 
 	memblock_merge_regions(type);
 	return 0;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] memblock: split checks whether a region should be skipped to a helper function
  2019-02-06 12:10 [PATCH 0/2] memblock: minor cleanups Mike Rapoport
  2019-02-06 12:10 ` [PATCH 1/2] memblock: remove memblock_{set,clear}_region_flags Mike Rapoport
@ 2019-02-06 12:10 ` Mike Rapoport
  1 sibling, 0 replies; 6+ messages in thread
From: Mike Rapoport @ 2019-02-06 12:10 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Michal Hocko, linux-mm, linux-kernel, Mike Rapoport

The __next_mem_range() and __next_mem_range_rev() duplucate the code that
checks whether a region should be skipped because of node or flags
incompatibility.

Split this code into a helper function.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 mm/memblock.c | 53 +++++++++++++++++++++++++----------------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index af5fe8e..f87d3ae 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -958,6 +958,29 @@ void __init_memblock __next_reserved_mem_region(u64 *idx,
 	*idx = ULLONG_MAX;
 }
 
+static bool should_skip_region(struct memblock_region *m, int nid, int flags)
+{
+	int m_nid = memblock_get_region_node(m);
+
+	/* only memory regions are associated with nodes, check it */
+	if (nid != NUMA_NO_NODE && nid != m_nid)
+		return true;
+
+	/* skip hotpluggable memory regions if needed */
+	if (movable_node_is_enabled() && memblock_is_hotpluggable(m))
+		return true;
+
+	/* if we want mirror memory skip non-mirror memory regions */
+	if ((flags & MEMBLOCK_MIRROR) && !memblock_is_mirror(m))
+		return true;
+
+	/* skip nomap memory unless we were asked for it explicitly */
+	if (!(flags & MEMBLOCK_NOMAP) && memblock_is_nomap(m))
+		return true;
+
+	return false;
+}
+
 /**
  * __next__mem_range - next function for for_each_free_mem_range() etc.
  * @idx: pointer to u64 loop variable
@@ -1005,20 +1028,7 @@ void __init_memblock __next_mem_range(u64 *idx, int nid,
 		phys_addr_t m_end = m->base + m->size;
 		int	    m_nid = memblock_get_region_node(m);
 
-		/* only memory regions are associated with nodes, check it */
-		if (nid != NUMA_NO_NODE && nid != m_nid)
-			continue;
-
-		/* skip hotpluggable memory regions if needed */
-		if (movable_node_is_enabled() && memblock_is_hotpluggable(m))
-			continue;
-
-		/* if we want mirror memory skip non-mirror memory regions */
-		if ((flags & MEMBLOCK_MIRROR) && !memblock_is_mirror(m))
-			continue;
-
-		/* skip nomap memory unless we were asked for it explicitly */
-		if (!(flags & MEMBLOCK_NOMAP) && memblock_is_nomap(m))
+		if (should_skip_region(m, nid, flags))
 			continue;
 
 		if (!type_b) {
@@ -1122,20 +1132,7 @@ void __init_memblock __next_mem_range_rev(u64 *idx, int nid,
 		phys_addr_t m_end = m->base + m->size;
 		int m_nid = memblock_get_region_node(m);
 
-		/* only memory regions are associated with nodes, check it */
-		if (nid != NUMA_NO_NODE && nid != m_nid)
-			continue;
-
-		/* skip hotpluggable memory regions if needed */
-		if (movable_node_is_enabled() && memblock_is_hotpluggable(m))
-			continue;
-
-		/* if we want mirror memory skip non-mirror memory regions */
-		if ((flags & MEMBLOCK_MIRROR) && !memblock_is_mirror(m))
-			continue;
-
-		/* skip nomap memory unless we were asked for it explicitly */
-		if (!(flags & MEMBLOCK_NOMAP) && memblock_is_nomap(m))
+		if (should_skip_region(m, nid, flags))
 			continue;
 
 		if (!type_b) {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] memblock: remove memblock_{set,clear}_region_flags
  2019-02-06 12:10 ` [PATCH 1/2] memblock: remove memblock_{set,clear}_region_flags Mike Rapoport
@ 2019-02-07 16:32     ` Souptick Joarder
  0 siblings, 0 replies; 6+ messages in thread
From: Souptick Joarder @ 2019-02-07 16:32 UTC (permalink / raw)
  To: Mike Rapoport; +Cc: Andrew Morton, Michal Hocko, Linux-MM, linux-kernel

On Wed, Feb 6, 2019 at 6:01 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> The memblock API provides dedicated helpers to set or clear a flag on a
> memory region, e.g. memblock_{mark,clear}_hotplug().
>
> The memblock_{set,clear}_region_flags() functions are used only by the
> memblock internal function that adjusts the region flags.
> Drop these functions and use open-coded implementation instead.
>
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
>  include/linux/memblock.h | 12 ------------
>  mm/memblock.c            |  9 ++++++---
>  2 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 71c9e32..32a9a6b 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -317,18 +317,6 @@ void __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
>         for_each_mem_range_rev(i, &memblock.memory, &memblock.reserved, \
>                                nid, flags, p_start, p_end, p_nid)
>
> -static inline void memblock_set_region_flags(struct memblock_region *r,
> -                                            enum memblock_flags flags)
> -{
> -       r->flags |= flags;
> -}
> -
> -static inline void memblock_clear_region_flags(struct memblock_region *r,
> -                                              enum memblock_flags flags)
> -{
> -       r->flags &= ~flags;
> -}
> -
>  #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
>  int memblock_set_node(phys_addr_t base, phys_addr_t size,
>                       struct memblock_type *type, int nid);
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 0151a5b..af5fe8e 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -851,11 +851,14 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base,
>         if (ret)
>                 return ret;
>
> -       for (i = start_rgn; i < end_rgn; i++)
> +       for (i = start_rgn; i < end_rgn; i++) {
> +               struct memblock_region *r = &type->regions[i];

Is it fine if we drop this memblock_region *r altogether ?

> +
>                 if (set)
> -                       memblock_set_region_flags(&type->regions[i], flag);
> +                       r->flags |= flag;
>                 else
> -                       memblock_clear_region_flags(&type->regions[i], flag);
> +                       r->flags &= ~flag;
> +       }
>
>         memblock_merge_regions(type);
>         return 0;
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] memblock: remove memblock_{set,clear}_region_flags
@ 2019-02-07 16:32     ` Souptick Joarder
  0 siblings, 0 replies; 6+ messages in thread
From: Souptick Joarder @ 2019-02-07 16:32 UTC (permalink / raw)
  To: Mike Rapoport; +Cc: Andrew Morton, Michal Hocko, Linux-MM, linux-kernel

On Wed, Feb 6, 2019 at 6:01 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> The memblock API provides dedicated helpers to set or clear a flag on a
> memory region, e.g. memblock_{mark,clear}_hotplug().
>
> The memblock_{set,clear}_region_flags() functions are used only by the
> memblock internal function that adjusts the region flags.
> Drop these functions and use open-coded implementation instead.
>
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
>  include/linux/memblock.h | 12 ------------
>  mm/memblock.c            |  9 ++++++---
>  2 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 71c9e32..32a9a6b 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -317,18 +317,6 @@ void __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
>         for_each_mem_range_rev(i, &memblock.memory, &memblock.reserved, \
>                                nid, flags, p_start, p_end, p_nid)
>
> -static inline void memblock_set_region_flags(struct memblock_region *r,
> -                                            enum memblock_flags flags)
> -{
> -       r->flags |= flags;
> -}
> -
> -static inline void memblock_clear_region_flags(struct memblock_region *r,
> -                                              enum memblock_flags flags)
> -{
> -       r->flags &= ~flags;
> -}
> -
>  #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
>  int memblock_set_node(phys_addr_t base, phys_addr_t size,
>                       struct memblock_type *type, int nid);
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 0151a5b..af5fe8e 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -851,11 +851,14 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base,
>         if (ret)
>                 return ret;
>
> -       for (i = start_rgn; i < end_rgn; i++)
> +       for (i = start_rgn; i < end_rgn; i++) {
> +               struct memblock_region *r = &type->regions[i];

Is it fine if we drop this memblock_region *r altogether ?

> +
>                 if (set)
> -                       memblock_set_region_flags(&type->regions[i], flag);
> +                       r->flags |= flag;
>                 else
> -                       memblock_clear_region_flags(&type->regions[i], flag);
> +                       r->flags &= ~flag;
> +       }
>
>         memblock_merge_regions(type);
>         return 0;
> --
> 2.7.4
>


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] memblock: remove memblock_{set,clear}_region_flags
  2019-02-07 16:32     ` Souptick Joarder
  (?)
@ 2019-02-08 10:04     ` Mike Rapoport
  -1 siblings, 0 replies; 6+ messages in thread
From: Mike Rapoport @ 2019-02-08 10:04 UTC (permalink / raw)
  To: Souptick Joarder; +Cc: Andrew Morton, Michal Hocko, Linux-MM, linux-kernel

On Thu, Feb 07, 2019 at 10:02:24PM +0530, Souptick Joarder wrote:
> On Wed, Feb 6, 2019 at 6:01 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> >
> > The memblock API provides dedicated helpers to set or clear a flag on a
> > memory region, e.g. memblock_{mark,clear}_hotplug().
> >
> > The memblock_{set,clear}_region_flags() functions are used only by the
> > memblock internal function that adjusts the region flags.
> > Drop these functions and use open-coded implementation instead.
> >
> > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > ---
> >  include/linux/memblock.h | 12 ------------
> >  mm/memblock.c            |  9 ++++++---
> >  2 files changed, 6 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index 71c9e32..32a9a6b 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -317,18 +317,6 @@ void __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
> >         for_each_mem_range_rev(i, &memblock.memory, &memblock.reserved, \
> >                                nid, flags, p_start, p_end, p_nid)
> >
> > -static inline void memblock_set_region_flags(struct memblock_region *r,
> > -                                            enum memblock_flags flags)
> > -{
> > -       r->flags |= flags;
> > -}
> > -
> > -static inline void memblock_clear_region_flags(struct memblock_region *r,
> > -                                              enum memblock_flags flags)
> > -{
> > -       r->flags &= ~flags;
> > -}
> > -
> >  #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> >  int memblock_set_node(phys_addr_t base, phys_addr_t size,
> >                       struct memblock_type *type, int nid);
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index 0151a5b..af5fe8e 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -851,11 +851,14 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base,
> >         if (ret)
> >                 return ret;
> >
> > -       for (i = start_rgn; i < end_rgn; i++)
> > +       for (i = start_rgn; i < end_rgn; i++) {
> > +               struct memblock_region *r = &type->regions[i];
> 
> Is it fine if we drop this memblock_region *r altogether ?

I prefer using a local variable to

	type->regions[i].flags
 
> > +
> >                 if (set)
> > -                       memblock_set_region_flags(&type->regions[i], flag);
> > +                       r->flags |= flag;
> >                 else
> > -                       memblock_clear_region_flags(&type->regions[i], flag);
> > +                       r->flags &= ~flag;
> > +       }
> >
> >         memblock_merge_regions(type);
> >         return 0;
> > --
> > 2.7.4
> >
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-08 10:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-06 12:10 [PATCH 0/2] memblock: minor cleanups Mike Rapoport
2019-02-06 12:10 ` [PATCH 1/2] memblock: remove memblock_{set,clear}_region_flags Mike Rapoport
2019-02-07 16:32   ` Souptick Joarder
2019-02-07 16:32     ` Souptick Joarder
2019-02-08 10:04     ` Mike Rapoport
2019-02-06 12:10 ` [PATCH 2/2] memblock: split checks whether a region should be skipped to a helper function Mike Rapoport

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.