[PATCH 1/3] powerpc/fadump: set an upper limit for the default memory reserved for fadump

[PATCH 1/3] powerpc/fadump: set an upper limit for the default memory reserved for fadump

[Hari Bathini]

When boot memory size for fadump is not specified, memory is reserved
for fadump based on system RAM size. As the system RAM size increases,
the memory reserved for fadump increases as well. This patch sets an
upper limit on the memory reserved for fadump, to avoid reserving
excess memory.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/fadump.h |    6 ++++++
 arch/powerpc/kernel/fadump.c      |    4 ++++
 2 files changed, 10 insertions(+)

diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
index b4407d0..2c3cb32 100644
--- a/arch/powerpc/include/asm/fadump.h
+++ b/arch/powerpc/include/asm/fadump.h
@@ -43,6 +43,12 @@
 #define MIN_BOOT_MEM	(((RMA_END < (0x1UL << 28)) ? (0x1UL << 28) : RMA_END) \
 			+ (0x1UL << 26))
 
+/*
+ * Maximum memory needed for fadump to boot up successfully. Use this as
+ * an upper limit for fadump so we don't endup reserving excess memory.
+ */
+#define MAX_BOOT_MEM	(0x1UL << 32)
+
 #define memblock_num_regions(memblock_type)	(memblock.memblock_type.cnt)
 
 #ifndef ELF_CORE_EFLAGS
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 3cb3b02a..d0af58b 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -225,6 +225,10 @@ static inline unsigned long fadump_calculate_reserve_size(void)
 	/* round it down in multiples of 256 */
 	size = size & ~0x0FFFFFFFUL;
 
+	/* Set an upper limit on the memory to be reserved */
+	if (size > MAX_BOOT_MEM)
+		size = MAX_BOOT_MEM;
+
 	/* Truncate to memory_limit. We don't want to over reserve the memory.*/
 	if (memory_limit && size > memory_limit)
 		size = memory_limit;

[PATCH 2/3] powerpc/fadump: add support to specify memory range based size

[Hari Bathini]

Currently, memory for fadump can be specified with fadump_reserve_mem=size,
where only a fixed size can be specified. This patch tries to extend this
syntax to support conditional reservation based on memory size, with the
below syntax:

	fadump_reserve_mem=<range1>:<size1>[,<range2>:<size2>,...]

This syntax helps using the same commandline parameter for different system
memory sizes.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/fadump.c |  127 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 118 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index d0af58b..a7fef3e 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -193,6 +193,121 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
 	return addr;
 }
 
+#define FADUMP_MEM_CMDLINE_PREFIX		"fadump_reserve_mem="
+
+static __init char *get_last_fadump_reserve_mem(void)
+{
+	char *p = boot_command_line, *fadump_cmdline = NULL;
+
+	/* find fadump_reserve_mem and use the last one if there are more */
+	p = strstr(p, FADUMP_MEM_CMDLINE_PREFIX);
+	while (p) {
+		fadump_cmdline = p;
+		p = strstr(p+1, FADUMP_MEM_CMDLINE_PREFIX);
+	}
+
+	return fadump_cmdline;
+}
+
+#define parse_fadump_print(fmt, arg...) \
+	printk(KERN_INFO "fadump_reserve_mem: "	fmt, ##arg)
+
+/*
+ * This function parses command line for fadump_reserve_mem=
+ *
+ * Supports the below two syntaxes:
+ *    1. fadump_reserve_mem=size
+ *    2. fadump_reserve_mem=ramsize-range:size[,...]
+ *
+ * Sets fw_dump.reserve_bootvar with the memory size
+ * provided, 0 otherwise
+ *
+ * The function returns -EINVAL on failure, 0 otherwise.
+ */
+static int __init parse_fadump_reserve_mem(void)
+{
+	char *cur, *tmp;
+	char *first_colon, *first_space;
+	char *fadump_cmdline;
+	unsigned long long system_ram;
+
+	fw_dump.reserve_bootvar = 0;
+	fadump_cmdline = get_last_fadump_reserve_mem();
+
+	/* when no fadump_reserve_mem= cmdline option is provided */
+	if (!fadump_cmdline)
+		return 0;
+
+	first_colon = strchr(fadump_cmdline, ':');
+	first_space = strchr(fadump_cmdline, ' ');
+	cur = fadump_cmdline + strlen(FADUMP_MEM_CMDLINE_PREFIX);
+
+	/* for fadump_reserve_mem=size cmdline syntax */
+	if (!first_colon || (first_space && (first_colon > first_space))) {
+		fw_dump.reserve_bootvar = memparse(cur, &cur);
+		return 0;
+	}
+
+	/* for fadump_reserve_mem=ramsize-range:size[,...] cmdline syntax */
+	system_ram = memblock_phys_mem_size();
+	/* for each entry of the comma-separated list */
+	do {
+		unsigned long long start, end = ULLONG_MAX, size;
+
+		/* get the start of the range */
+		start = memparse(cur, &tmp);
+		if (cur == tmp) {
+			parse_fadump_print("Memory value expected\n");
+			return -EINVAL;
+		}
+		cur = tmp;
+		if (*cur != '-') {
+			parse_fadump_print("'-' expected\n");
+			return -EINVAL;
+		}
+		cur++;
+
+		/* if no ':' is here, than we read the end */
+		if (*cur != ':') {
+			end = memparse(cur, &tmp);
+			if (cur == tmp) {
+				parse_fadump_print("Memory value expected\n");
+				return -EINVAL;
+			}
+			cur = tmp;
+			if (end <= start) {
+				parse_fadump_print("end <= start\n");
+				return -EINVAL;
+			}
+		}
+
+		if (*cur != ':') {
+			parse_fadump_print("':' expected\n");
+			return -EINVAL;
+		}
+		cur++;
+
+		size = memparse(cur, &tmp);
+		if (cur == tmp) {
+			parse_fadump_print("Memory value expected\n");
+			return -EINVAL;
+		}
+		cur = tmp;
+		if (size >= system_ram) {
+			parse_fadump_print("invalid size\n");
+			return -EINVAL;
+		}
+
+		/* match ? */
+		if (system_ram >= start && system_ram < end) {
+			fw_dump.reserve_bootvar = size;
+			break;
+		}
+	} while (*cur++ == ',');
+
+	return 0;
+}
+
 /**
  * fadump_calculate_reserve_size(): reserve variable boot area 5% of System RAM
  *
@@ -212,6 +327,9 @@ static inline unsigned long fadump_calculate_reserve_size(void)
 {
 	unsigned long size;
 
+	/* sets fw_dump.reserve_bootvar */
+	parse_fadump_reserve_mem();
+
 	/*
 	 * Check if the size is specified through fadump_reserve_mem= cmdline
 	 * option. If yes, then use that.
@@ -352,15 +470,6 @@ static int __init early_fadump_param(char *p)
 }
 early_param("fadump", early_fadump_param);
 
-/* Look for fadump_reserve_mem= cmdline option */
-static int __init early_fadump_reserve_mem(char *p)
-{
-	if (p)
-		fw_dump.reserve_bootvar = memparse(p, &p);
-	return 0;
-}
-early_param("fadump_reserve_mem", early_fadump_reserve_mem);
-
 static void register_fw_dump(struct fadump_mem_struct *fdm)
 {
 	int rc;

[PATCH 3/3] powerpc/fadump: add support for fadump_nr_cpus= parameter

[Hari Bathini]

Kernel parameter 'nr_cpus' can be used to limit the maximum number
of processors that an SMP kernel could support. This patch extends
this to fadump by introducing 'fadump_nr_cpus' parameter that can
help in booting fadump kernel on a lower memory footprint.

Suggested-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/fadump.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index a7fef3e..c75783c 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -470,6 +470,28 @@ static int __init early_fadump_param(char *p)
 }
 early_param("fadump", early_fadump_param);
 
+/* Look for fadump_nr_cpus= cmdline option. */
+static int __init early_fadump_nrcpus(char *p)
+{
+	int nr_cpus;
+
+	/*
+	 * fadump_nr_cpus parameter is only applicable on a
+	 * fadump active kernel. This is to reduce memory
+	 * needed to boot a fadump active kernel.
+	 * So, check if we are booting after crash.
+	 */
+	if (!is_fadump_active())
+		return 0;
+
+	get_option(&p, &nr_cpus);
+	if (nr_cpus > 0 && nr_cpus < nr_cpu_ids)
+		nr_cpu_ids = nr_cpus;
+
+	return 0;
+}
+early_param("fadump_nr_cpus", early_fadump_nrcpus);
+
 static void register_fw_dump(struct fadump_mem_struct *fdm)
 {
 	int rc;

Re: [2/3] powerpc/fadump: add support to specify memory range based size

[Michael Ellerman]

On Fri, 2016-06-05 at 11:50:37 UTC, Hari Bathini wrote:
> Currently, memory for fadump can be specified with fadump_reserve_mem=size,
> where only a fixed size can be specified. This patch tries to extend this
> syntax to support conditional reservation based on memory size, with the
> below syntax:
> 
> 	fadump_reserve_mem=<range1>:<size1>[,<range2>:<size2>,...]
> 
> This syntax helps using the same commandline parameter for different system
> memory sizes.

This is basically using the crashkernel= syntax right?

So can we please reuse the crashkernel= parsing code?

cheers

Re: [3/3] powerpc/fadump: add support for fadump_nr_cpus= parameter

[Michael Ellerman]

On Fri, 2016-06-05 at 11:51:08 UTC, Hari Bathini wrote:
> Kernel parameter 'nr_cpus' can be used to limit the maximum number
> of processors that an SMP kernel could support. This patch extends
> this to fadump by introducing 'fadump_nr_cpus' parameter that can
> help in booting fadump kernel on a lower memory footprint.

Is there really no other way to do this? I really hate adding new, single use
only command line parameters.

cheers

Re: [3/3] powerpc/fadump: add support for fadump_nr_cpus= parameter

[Hari Bathini]

On 05/07/2016 09:42 AM, Michael Ellerman wrote:
> On Fri, 2016-06-05 at 11:51:08 UTC, Hari Bathini wrote:
>> Kernel parameter 'nr_cpus' can be used to limit the maximum number
>> of processors that an SMP kernel could support. This patch extends
>> this to fadump by introducing 'fadump_nr_cpus' parameter that can
>> help in booting fadump kernel on a lower memory footprint.
> Is there really no other way to do this? I really hate adding new, single use
> only command line parameters.

Hmmm.. only alternative I can think about is enforcing a certain nr_cpu_ids
value whenever fadump is active, but that doesn't sound right..
Any suggestions?

Thanks
Hari

> cheers
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [2/3] powerpc/fadump: add support to specify memory range based size

[Hari Bathini]

On 05/07/2016 09:41 AM, Michael Ellerman wrote:
> On Fri, 2016-06-05 at 11:50:37 UTC, Hari Bathini wrote:
>> Currently, memory for fadump can be specified with fadump_reserve_mem=size,
>> where only a fixed size can be specified. This patch tries to extend this
>> syntax to support conditional reservation based on memory size, with the
>> below syntax:
>>
>> 	fadump_reserve_mem=<range1>:<size1>[,<range2>:<size2>,...]
>>
>> This syntax helps using the same commandline parameter for different system
>> memory sizes.
> This is basically using the crashkernel= syntax right?

Yep. One of the typical crashkernel syntax..

> So can we please reuse the crashkernel= parsing code?

but crashkernel has a few other variants which don't make sense
for fadump. To reuse the crashkernel parsing code for fadump,
it needs little bit of refactoring. Will try to do that and respin..

> cheers
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev