Re: [PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
On Thu, Jul 02, 2020 at 09:48:52PM -0700, Andrew Morton wrote: > On Thu, 2 Jul 2020 09:23:10 +0200 David Hildenbrand wrote: > > > >>> --- > > >>> arch/s390/kernel/crash_dump.c | 6 ++-- > > >>> include/linux/memblock.h | 28 ++--- > > >>> mm/memblock.c | 57 ++- > > >>> 3 files changed, 55 insertions(+), 36 deletions(-) > > > > > > So I guess this should go via the s390 tree, since the second patch of > > > this series can go only upstream if both this patch and a patch which > > > is currently only on our features are merged before. > > > > > > Any objections? > > > > @Andrew, fine with you if this goes via the s390 tree? > > Sure, please go ahead. Ok, applied both patches. Thanks!
Re: [PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
On Thu, 2 Jul 2020 09:23:10 +0200 David Hildenbrand wrote: > >>> --- > >>> arch/s390/kernel/crash_dump.c | 6 ++-- > >>> include/linux/memblock.h | 28 ++--- > >>> mm/memblock.c | 57 ++- > >>> 3 files changed, 55 insertions(+), 36 deletions(-) > > > > So I guess this should go via the s390 tree, since the second patch of > > this series can go only upstream if both this patch and a patch which > > is currently only on our features are merged before. > > > > Any objections? > > @Andrew, fine with you if this goes via the s390 tree? Sure, please go ahead.
Re: [PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
On 01.07.20 17:31, Heiko Carstens wrote: > On Wed, Jul 01, 2020 at 06:06:43PM +0300, Mike Rapoport wrote: >> Hi David, >> >> On Wed, Jul 01, 2020 at 04:18:29PM +0200, David Hildenbrand wrote: >>> "physmem" in the memblock allocator is somewhat weird: it's not actually >>> used for allocation, it's simply information collected during boot, which >>> describes the unmodified physical memory map at boot time, without any >>> standby/hotplugged memory. It's only used on s390x and is currently the >>> only reason s390x keeps using CONFIG_ARCH_KEEP_MEMBLOCK. >>> >>> Physmem isn't numa aware and current users don't specify any flags. Let's >>> hide it from the user, exposing only for_each_physmem(), and simplify. The >>> interface for physmem is now really minimalistic: >>> - memblock_physmem_add() to add ranges >>> - for_each_physmem() / __next_physmem_range() to walk physmem ranges >>> >>> Don't place it into an __init section and don't discard it without >>> CONFIG_ARCH_KEEP_MEMBLOCK. As we're reusing __next_mem_range(), remove >>> the __meminit notifier to avoid section mismatch warnings once >>> CONFIG_ARCH_KEEP_MEMBLOCK is no longer used with >>> CONFIG_HAVE_MEMBLOCK_PHYS_MAP. >>> >>> While fixing up the documentation, sneak in some related cleanups. We can >>> stop setting CONFIG_HAVE_MEMBLOCK_PHYS_MAP for s390x next. >> >> As you noted in the previous version it should have been >> CONFIG_ARCH_KEEP_MEMBLOCK ;-) >> >>> Cc: Heiko Carstens >>> Cc: Vasily Gorbik >>> Cc: Christian Borntraeger >>> Cc: Mike Rapoport >>> Cc: Andrew Morton >>> Signed-off-by: David Hildenbrand >> >> Reviewed-by: Mike Rapoport >> >>> --- >>> arch/s390/kernel/crash_dump.c | 6 ++-- >>> include/linux/memblock.h | 28 ++--- >>> mm/memblock.c | 57 ++- >>> 3 files changed, 55 insertions(+), 36 deletions(-) > > So I guess this should go via the s390 tree, since the second patch of > this series can go only upstream if both this patch and a patch which > is currently only on our features are merged before. > > Any objections? @Andrew, fine with you if this goes via the s390 tree? -- Thanks, David / dhildenb
Re: [PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
On Wed, Jul 01, 2020 at 05:31:57PM +0200, Heiko Carstens wrote: > On Wed, Jul 01, 2020 at 06:06:43PM +0300, Mike Rapoport wrote: > > Hi David, > > > > On Wed, Jul 01, 2020 at 04:18:29PM +0200, David Hildenbrand wrote: > > > "physmem" in the memblock allocator is somewhat weird: it's not actually > > > used for allocation, it's simply information collected during boot, which > > > describes the unmodified physical memory map at boot time, without any > > > standby/hotplugged memory. It's only used on s390x and is currently the > > > only reason s390x keeps using CONFIG_ARCH_KEEP_MEMBLOCK. > > > > > > Physmem isn't numa aware and current users don't specify any flags. Let's > > > hide it from the user, exposing only for_each_physmem(), and simplify. The > > > interface for physmem is now really minimalistic: > > > - memblock_physmem_add() to add ranges > > > - for_each_physmem() / __next_physmem_range() to walk physmem ranges > > > > > > Don't place it into an __init section and don't discard it without > > > CONFIG_ARCH_KEEP_MEMBLOCK. As we're reusing __next_mem_range(), remove > > > the __meminit notifier to avoid section mismatch warnings once > > > CONFIG_ARCH_KEEP_MEMBLOCK is no longer used with > > > CONFIG_HAVE_MEMBLOCK_PHYS_MAP. > > > > > > While fixing up the documentation, sneak in some related cleanups. We can > > > stop setting CONFIG_HAVE_MEMBLOCK_PHYS_MAP for s390x next. > > > > As you noted in the previous version it should have been > > CONFIG_ARCH_KEEP_MEMBLOCK ;-) > > > > > Cc: Heiko Carstens > > > Cc: Vasily Gorbik > > > Cc: Christian Borntraeger > > > Cc: Mike Rapoport > > > Cc: Andrew Morton > > > Signed-off-by: David Hildenbrand > > > > Reviewed-by: Mike Rapoport > > > > > --- > > > arch/s390/kernel/crash_dump.c | 6 ++-- > > > include/linux/memblock.h | 28 ++--- > > > mm/memblock.c | 57 ++- > > > 3 files changed, 55 insertions(+), 36 deletions(-) > > So I guess this should go via the s390 tree, since the second patch of > this series can go only upstream if both this patch and a patch which > is currently only on our features are merged before. > > Any objections? Not from my side. -- Sincerely yours, Mike.
Re: [PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
> >> While fixing up the documentation, sneak in some related cleanups. We can > >> stop setting CONFIG_HAVE_MEMBLOCK_PHYS_MAP for s390x next. > > As you noted in the previous version it should have been > > CONFIG_ARCH_KEEP_MEMBLOCK ;-) > Grml :) maybe maintainers can fix that up when applying in case > there are no other comments. Sure, just need to know how to handle this. :)
Re: [PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
> Am 01.07.2020 um 17:07 schrieb Mike Rapoport : > > Hi David, > >> On Wed, Jul 01, 2020 at 04:18:29PM +0200, David Hildenbrand wrote: >> "physmem" in the memblock allocator is somewhat weird: it's not actually >> used for allocation, it's simply information collected during boot, which >> describes the unmodified physical memory map at boot time, without any >> standby/hotplugged memory. It's only used on s390x and is currently the >> only reason s390x keeps using CONFIG_ARCH_KEEP_MEMBLOCK. >> >> Physmem isn't numa aware and current users don't specify any flags. Let's >> hide it from the user, exposing only for_each_physmem(), and simplify. The >> interface for physmem is now really minimalistic: >> - memblock_physmem_add() to add ranges >> - for_each_physmem() / __next_physmem_range() to walk physmem ranges >> >> Don't place it into an __init section and don't discard it without >> CONFIG_ARCH_KEEP_MEMBLOCK. As we're reusing __next_mem_range(), remove >> the __meminit notifier to avoid section mismatch warnings once >> CONFIG_ARCH_KEEP_MEMBLOCK is no longer used with >> CONFIG_HAVE_MEMBLOCK_PHYS_MAP. >> >> While fixing up the documentation, sneak in some related cleanups. We can >> stop setting CONFIG_HAVE_MEMBLOCK_PHYS_MAP for s390x next. > > As you noted in the previous version it should have been > CONFIG_ARCH_KEEP_MEMBLOCK ;-) Grml :) maybe maintainers can fix that up when applying in case there are no other comments. Thanks Mike for the fast review!
Re: [PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
On Wed, Jul 01, 2020 at 06:06:43PM +0300, Mike Rapoport wrote: > Hi David, > > On Wed, Jul 01, 2020 at 04:18:29PM +0200, David Hildenbrand wrote: > > "physmem" in the memblock allocator is somewhat weird: it's not actually > > used for allocation, it's simply information collected during boot, which > > describes the unmodified physical memory map at boot time, without any > > standby/hotplugged memory. It's only used on s390x and is currently the > > only reason s390x keeps using CONFIG_ARCH_KEEP_MEMBLOCK. > > > > Physmem isn't numa aware and current users don't specify any flags. Let's > > hide it from the user, exposing only for_each_physmem(), and simplify. The > > interface for physmem is now really minimalistic: > > - memblock_physmem_add() to add ranges > > - for_each_physmem() / __next_physmem_range() to walk physmem ranges > > > > Don't place it into an __init section and don't discard it without > > CONFIG_ARCH_KEEP_MEMBLOCK. As we're reusing __next_mem_range(), remove > > the __meminit notifier to avoid section mismatch warnings once > > CONFIG_ARCH_KEEP_MEMBLOCK is no longer used with > > CONFIG_HAVE_MEMBLOCK_PHYS_MAP. > > > > While fixing up the documentation, sneak in some related cleanups. We can > > stop setting CONFIG_HAVE_MEMBLOCK_PHYS_MAP for s390x next. > > As you noted in the previous version it should have been > CONFIG_ARCH_KEEP_MEMBLOCK ;-) > > > Cc: Heiko Carstens > > Cc: Vasily Gorbik > > Cc: Christian Borntraeger > > Cc: Mike Rapoport > > Cc: Andrew Morton > > Signed-off-by: David Hildenbrand > > Reviewed-by: Mike Rapoport > > > --- > > arch/s390/kernel/crash_dump.c | 6 ++-- > > include/linux/memblock.h | 28 ++--- > > mm/memblock.c | 57 ++- > > 3 files changed, 55 insertions(+), 36 deletions(-) So I guess this should go via the s390 tree, since the second patch of this series can go only upstream if both this patch and a patch which is currently only on our features are merged before. Any objections?
Re: [PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
Hi David,
On Wed, Jul 01, 2020 at 04:18:29PM +0200, David Hildenbrand wrote:
> "physmem" in the memblock allocator is somewhat weird: it's not actually
> used for allocation, it's simply information collected during boot, which
> describes the unmodified physical memory map at boot time, without any
> standby/hotplugged memory. It's only used on s390x and is currently the
> only reason s390x keeps using CONFIG_ARCH_KEEP_MEMBLOCK.
>
> Physmem isn't numa aware and current users don't specify any flags. Let's
> hide it from the user, exposing only for_each_physmem(), and simplify. The
> interface for physmem is now really minimalistic:
> - memblock_physmem_add() to add ranges
> - for_each_physmem() / __next_physmem_range() to walk physmem ranges
>
> Don't place it into an __init section and don't discard it without
> CONFIG_ARCH_KEEP_MEMBLOCK. As we're reusing __next_mem_range(), remove
> the __meminit notifier to avoid section mismatch warnings once
> CONFIG_ARCH_KEEP_MEMBLOCK is no longer used with
> CONFIG_HAVE_MEMBLOCK_PHYS_MAP.
>
> While fixing up the documentation, sneak in some related cleanups. We can
> stop setting CONFIG_HAVE_MEMBLOCK_PHYS_MAP for s390x next.
As you noted in the previous version it should have been
CONFIG_ARCH_KEEP_MEMBLOCK ;-)
> Cc: Heiko Carstens
> Cc: Vasily Gorbik
> Cc: Christian Borntraeger
> Cc: Mike Rapoport
> Cc: Andrew Morton
> Signed-off-by: David Hildenbrand
Reviewed-by: Mike Rapoport
> ---
> arch/s390/kernel/crash_dump.c | 6 ++--
> include/linux/memblock.h | 28 ++---
> mm/memblock.c | 57 ++-
> 3 files changed, 55 insertions(+), 36 deletions(-)
>
> diff --git a/arch/s390/kernel/crash_dump.c b/arch/s390/kernel/crash_dump.c
> index f96a5857bbfde..c42ce348103cc 100644
> --- a/arch/s390/kernel/crash_dump.c
> +++ b/arch/s390/kernel/crash_dump.c
> @@ -549,8 +549,7 @@ static int get_mem_chunk_cnt(void)
> int cnt = 0;
> u64 idx;
>
> - for_each_mem_range(idx, &memblock.physmem, &oldmem_type, NUMA_NO_NODE,
> -MEMBLOCK_NONE, NULL, NULL, NULL)
> + for_each_physmem_range(idx, &oldmem_type, NULL, NULL)
> cnt++;
> return cnt;
> }
> @@ -563,8 +562,7 @@ static void loads_init(Elf64_Phdr *phdr, u64 loads_offset)
> phys_addr_t start, end;
> u64 idx;
>
> - for_each_mem_range(idx, &memblock.physmem, &oldmem_type, NUMA_NO_NODE,
> -MEMBLOCK_NONE, &start, &end, NULL) {
> + for_each_physmem_range(idx, &oldmem_type, &start, &end) {
> phdr->p_filesz = end - start;
> phdr->p_type = PT_LOAD;
> phdr->p_offset = start;
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 017fae833d4ae..9d925db0d3550 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -77,16 +77,12 @@ struct memblock_type {
> * @current_limit: physical address of the current allocation limit
> * @memory: usable memory regions
> * @reserved: reserved memory regions
> - * @physmem: all physical memory
> */
> struct memblock {
> bool bottom_up; /* is bottom up direction? */
> phys_addr_t current_limit;
> struct memblock_type memory;
> struct memblock_type reserved;
> -#ifdef CONFIG_HAVE_MEMBLOCK_PHYS_MAP
> - struct memblock_type physmem;
> -#endif
> };
>
> extern struct memblock memblock;
> @@ -145,6 +141,30 @@ void __next_reserved_mem_region(u64 *idx, phys_addr_t
> *out_start,
>
> void __memblock_free_late(phys_addr_t base, phys_addr_t size);
>
> +#ifdef CONFIG_HAVE_MEMBLOCK_PHYS_MAP
> +static inline void __next_physmem_range(u64 *idx, struct memblock_type *type,
> + phys_addr_t *out_start,
> + phys_addr_t *out_end)
> +{
> + extern struct memblock_type physmem;
> +
> + __next_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE, &physmem, type,
> + out_start, out_end, NULL);
> +}
> +
> +/**
> + * for_each_physmem_range - iterate through physmem areas not included in
> type.
> + * @i: u64 used as loop variable
> + * @type: ptr to memblock_type which excludes from the iteration, can be
> %NULL
> + * @p_start: ptr to phys_addr_t for start address of the range, can be %NULL
> + * @p_end: ptr to phys_addr_t for end address of the range, can be %NULL
> + */
> +#define for_each_physmem_range(i, type, p_start, p_end)
> \
> + for (i = 0, __next_physmem_range(&i, type, p_start, p_end); \
> + i != (u64)ULLONG_MAX; \
> + __next_physmem_range(&i, type, p_start, p_end))
> +#endif /* CONFIG_HAVE_MEMBLOCK_PHYS_MAP */
> +
> /**
> * for_each_mem_range - iterate through memblock areas from type_a and not
> * included in type_b. Or just type_a if type_b is NULL.
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 39aceafc57f66..45f198750be92 10064
[PATCH v2 1/2] mm/memblock: expose only miminal interface to add/walk physmem
"physmem" in the memblock allocator is somewhat weird: it's not actually
used for allocation, it's simply information collected during boot, which
describes the unmodified physical memory map at boot time, without any
standby/hotplugged memory. It's only used on s390x and is currently the
only reason s390x keeps using CONFIG_ARCH_KEEP_MEMBLOCK.
Physmem isn't numa aware and current users don't specify any flags. Let's
hide it from the user, exposing only for_each_physmem(), and simplify. The
interface for physmem is now really minimalistic:
- memblock_physmem_add() to add ranges
- for_each_physmem() / __next_physmem_range() to walk physmem ranges
Don't place it into an __init section and don't discard it without
CONFIG_ARCH_KEEP_MEMBLOCK. As we're reusing __next_mem_range(), remove
the __meminit notifier to avoid section mismatch warnings once
CONFIG_ARCH_KEEP_MEMBLOCK is no longer used with
CONFIG_HAVE_MEMBLOCK_PHYS_MAP.
While fixing up the documentation, sneak in some related cleanups. We can
stop setting CONFIG_HAVE_MEMBLOCK_PHYS_MAP for s390x next.
Cc: Heiko Carstens
Cc: Vasily Gorbik
Cc: Christian Borntraeger
Cc: Mike Rapoport
Cc: Andrew Morton
Signed-off-by: David Hildenbrand
---
arch/s390/kernel/crash_dump.c | 6 ++--
include/linux/memblock.h | 28 ++---
mm/memblock.c | 57 ++-
3 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/arch/s390/kernel/crash_dump.c b/arch/s390/kernel/crash_dump.c
index f96a5857bbfde..c42ce348103cc 100644
--- a/arch/s390/kernel/crash_dump.c
+++ b/arch/s390/kernel/crash_dump.c
@@ -549,8 +549,7 @@ static int get_mem_chunk_cnt(void)
int cnt = 0;
u64 idx;
- for_each_mem_range(idx, &memblock.physmem, &oldmem_type, NUMA_NO_NODE,
- MEMBLOCK_NONE, NULL, NULL, NULL)
+ for_each_physmem_range(idx, &oldmem_type, NULL, NULL)
cnt++;
return cnt;
}
@@ -563,8 +562,7 @@ static void loads_init(Elf64_Phdr *phdr, u64 loads_offset)
phys_addr_t start, end;
u64 idx;
- for_each_mem_range(idx, &memblock.physmem, &oldmem_type, NUMA_NO_NODE,
- MEMBLOCK_NONE, &start, &end, NULL) {
+ for_each_physmem_range(idx, &oldmem_type, &start, &end) {
phdr->p_filesz = end - start;
phdr->p_type = PT_LOAD;
phdr->p_offset = start;
diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 017fae833d4ae..9d925db0d3550 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -77,16 +77,12 @@ struct memblock_type {
* @current_limit: physical address of the current allocation limit
* @memory: usable memory regions
* @reserved: reserved memory regions
- * @physmem: all physical memory
*/
struct memblock {
bool bottom_up; /* is bottom up direction? */
phys_addr_t current_limit;
struct memblock_type memory;
struct memblock_type reserved;
-#ifdef CONFIG_HAVE_MEMBLOCK_PHYS_MAP
- struct memblock_type physmem;
-#endif
};
extern struct memblock memblock;
@@ -145,6 +141,30 @@ void __next_reserved_mem_region(u64 *idx, phys_addr_t
*out_start,
void __memblock_free_late(phys_addr_t base, phys_addr_t size);
+#ifdef CONFIG_HAVE_MEMBLOCK_PHYS_MAP
+static inline void __next_physmem_range(u64 *idx, struct memblock_type *type,
+ phys_addr_t *out_start,
+ phys_addr_t *out_end)
+{
+ extern struct memblock_type physmem;
+
+ __next_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE, &physmem, type,
+out_start, out_end, NULL);
+}
+
+/**
+ * for_each_physmem_range - iterate through physmem areas not included in type.
+ * @i: u64 used as loop variable
+ * @type: ptr to memblock_type which excludes from the iteration, can be %NULL
+ * @p_start: ptr to phys_addr_t for start address of the range, can be %NULL
+ * @p_end: ptr to phys_addr_t for end address of the range, can be %NULL
+ */
+#define for_each_physmem_range(i, type, p_start, p_end)
\
+ for (i = 0, __next_physmem_range(&i, type, p_start, p_end); \
+i != (u64)ULLONG_MAX; \
+__next_physmem_range(&i, type, p_start, p_end))
+#endif /* CONFIG_HAVE_MEMBLOCK_PHYS_MAP */
+
/**
* for_each_mem_range - iterate through memblock areas from type_a and not
* included in type_b. Or just type_a if type_b is NULL.
diff --git a/mm/memblock.c b/mm/memblock.c
index 39aceafc57f66..45f198750be92 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -44,19 +44,20 @@
* in the system, for instance when the memory is restricted with
* ``mem=`` command line parameter
* * ``reserved`` - describes the regions that were allocated
- * * ``physmap`` - describes the actual physical memory regardless of
- * the possible restrictions; the ``physmap`` type is only ava
