[PATCH v3 0/2] powerpc/fadump: support memory range syntax for fadump memory reservation

[PATCH v3 0/2] powerpc/fadump: support memory range syntax for fadump memory reservation

[Hari Bathini]

This patchset adds support to input system memory range based memory size
for fadump reservation. The crashkernel parameter already supports such
syntax. The first patch refactors the parsing code of crashkernel parameter
for reuse. The second patch uses the newly refactored parsing code to reserve
memory for fadump based on system memory size.

---

Hari Bathini (2):
      kexec: refactor code parsing size based on memory range
      powerpc/fadump: parse fadump reserve memory size based on memory range


 arch/powerpc/kernel/fadump.c |   63 ++++++++++++++++++++++---
 include/linux/kernel.h       |    5 ++
 kernel/kexec_core.c          |   63 ++-----------------------
 lib/cmdline.c                |  104 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 168 insertions(+), 67 deletions(-)

[PATCH v3 1/2] kexec: refactor code parsing size based on memory range

[Hari Bathini]

crashkernel parameter supports different syntaxes to specify the amount
of memory to be reserved for kdump kernel. Below is one of the supported
syntaxes that needs parsing to find the memory size to reserve, based on
memory range:

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

While such parsing is implemented for crashkernel parameter, it applies
to other parameters, like fadump_reserve_mem=, which could use similar
syntax. This patch moves crashkernel's parsing code for above syntax to
to kernel/params.c file for reuse. Two functions is_param_range_based()
and parse_mem_range_size() are added to kernel/params.c file for this
purpose.

Any parameter that uses the above syntax can use is_param_range_based()
function to validate the syntax and parse_mem_range_size() function to
get the parsed memory size. While some code is moved to kernel/params.c
file, there is no change functionality wise in parsing the crashkernel
parameter.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---

Changes from v2:
1. Moved the code to lib/cmdline.c instead of kernel/params.c


 include/linux/kernel.h |    5 ++
 kernel/kexec_core.c    |   63 ++---------------------------
 lib/cmdline.c          |  104 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 114 insertions(+), 58 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d96a611..39ff869 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -435,6 +435,11 @@ extern char *get_options(const char *str, int nints, int *ints);
 extern unsigned long long memparse(const char *ptr, char **retptr);
 extern bool parse_option_str(const char *str, const char *option);
 
+extern bool __init is_colon_in_param(const char *cmdline);
+extern unsigned long long __init parse_mem_range_size(const char *param,
+						      char **str,
+						      unsigned long long system_ram);
+
 extern int core_kernel_text(unsigned long addr);
 extern int core_kernel_data(unsigned long addr);
 extern int __kernel_text_address(unsigned long addr);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..152c4c1 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1104,59 +1104,9 @@ static int __init parse_crashkernel_mem(char *cmdline,
 	char *cur = cmdline, *tmp;
 
 	/* 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) {
-			pr_warn("crashkernel: Memory value expected\n");
-			return -EINVAL;
-		}
-		cur = tmp;
-		if (*cur != '-') {
-			pr_warn("crashkernel: '-' expected\n");
-			return -EINVAL;
-		}
-		cur++;
-
-		/* if no ':' is here, than we read the end */
-		if (*cur != ':') {
-			end = memparse(cur, &tmp);
-			if (cur == tmp) {
-				pr_warn("crashkernel: Memory value expected\n");
-				return -EINVAL;
-			}
-			cur = tmp;
-			if (end <= start) {
-				pr_warn("crashkernel: end <= start\n");
-				return -EINVAL;
-			}
-		}
-
-		if (*cur != ':') {
-			pr_warn("crashkernel: ':' expected\n");
-			return -EINVAL;
-		}
-		cur++;
-
-		size = memparse(cur, &tmp);
-		if (cur == tmp) {
-			pr_warn("Memory value expected\n");
-			return -EINVAL;
-		}
-		cur = tmp;
-		if (size >= system_ram) {
-			pr_warn("crashkernel: invalid size\n");
-			return -EINVAL;
-		}
-
-		/* match ? */
-		if (system_ram >= start && system_ram < end) {
-			*crash_size = size;
-			break;
-		}
-	} while (*cur++ == ',');
+	*crash_size = parse_mem_range_size("crashkernel", &cur, system_ram);
+	if (cur == cmdline)
+		return -EINVAL;
 
 	if (*crash_size > 0) {
 		while (*cur && *cur != ' ' && *cur != '@')
@@ -1293,7 +1243,6 @@ static int __init __parse_crashkernel(char *cmdline,
 			     const char *name,
 			     const char *suffix)
 {
-	char	*first_colon, *first_space;
 	char	*ck_cmdline;
 
 	BUG_ON(!crash_size || !crash_base);
@@ -1311,12 +1260,10 @@ static int __init __parse_crashkernel(char *cmdline,
 		return parse_crashkernel_suffix(ck_cmdline, crash_size,
 				suffix);
 	/*
-	 * if the commandline contains a ':', then that's the extended
+	 * if the parameter is range based, then that's the extended
 	 * syntax -- if not, it must be the classic syntax
 	 */
-	first_colon = strchr(ck_cmdline, ':');
-	first_space = strchr(ck_cmdline, ' ');
-	if (first_colon && (!first_space || first_colon < first_space))
+	if (is_colon_in_param(ck_cmdline))
 		return parse_crashkernel_mem(ck_cmdline, system_ram,
 				crash_size, crash_base);
 
diff --git a/lib/cmdline.c b/lib/cmdline.c
index 8f13cf7..cbe22ea 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -189,3 +189,107 @@ bool parse_option_str(const char *str, const char *option)
 
 	return false;
 }
+
+/*
+ * is_colon_in_param - check if current parameter has colon in it
+ * @cmdline: points to the parameter to check
+ *
+ * This function checks if the current parameter has a colon in it.
+ * Can be used for parameters that are range based or likewise.
+ *
+ * Returns true when the current paramer is has colon, false otherwise
+ */
+bool __init is_colon_in_param(const char *cmdline)
+{
+	char    *first_colon, *first_space;
+
+	first_colon = strchr(cmdline, ':');
+	first_space = strchr(cmdline, ' ');
+	if (first_colon && (!first_space || first_colon < first_space))
+		return true;
+
+	return false;
+}
+
+/*
+ * parse_mem_range_size - parse size based on memory range
+ * @param:  the parameter being parsed
+ * @str: (input)  where parsing begins
+ *                expected format - <range1>:<size1>[,<range2>:<size2>,...]
+ *                Range is specified with a number (potentially suffixed with
+ *                K, M, G, T, P, E), signifying an inclusive start size,
+ *                followed by a hypen and possibly another number signifying
+ *                an exclusive end size
+ *                Eg. : 4G-8G:512M,8G-32G:1024M,32G-128G:2048M,128G-:4096M
+ *       (output) On success - next char after parse completes
+ *                On failure - unchanged
+ * @system_ram: system ram size to check memory range against
+ *
+ * Returns the memory size on success and 0 on failure
+ */
+unsigned long long __init parse_mem_range_size(const char *param,
+					       char **str,
+					       unsigned long long system_ram)
+{
+	char *cur = *str, *tmp;
+	unsigned long long mem_size = 0;
+
+	/* 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) {
+			printk(KERN_INFO "%s: memory value expected\n", param);
+			return mem_size;
+		}
+		cur = tmp;
+		if (*cur != '-') {
+			printk(KERN_INFO "%s: '-' expected\n", param);
+			return mem_size;
+		}
+		cur++;
+
+		/* if no ':' is here, than we read the end */
+		if (*cur != ':') {
+			end = memparse(cur, &tmp);
+			if (cur == tmp) {
+				printk(KERN_INFO "%s: memory value expected\n",
+					param);
+				return mem_size;
+			}
+			cur = tmp;
+			if (end <= start) {
+				printk(KERN_INFO "%s: end <= start\n", param);
+				return mem_size;
+			}
+		}
+
+		if (*cur != ':') {
+			printk(KERN_INFO "%s: ':' expected\n", param);
+			return mem_size;
+		}
+		cur++;
+
+		size = memparse(cur, &tmp);
+		if (cur == tmp) {
+			printk(KERN_INFO "%s: memory value expected\n", param);
+			return mem_size;
+		}
+		cur = tmp;
+		if (size >= system_ram) {
+			printk(KERN_INFO "%s: invalid size\n", param);
+			return mem_size;
+		}
+
+		/* match ? */
+		if (system_ram >= start && system_ram < end) {
+			mem_size = size;
+			*str = cur;
+			break;
+		}
+	} while (*cur++ == ',');
+
+	return mem_size;
+}

[PATCH v3 2/2] powerpc/fadump: parse fadump reserve memory size based on memory range

[Hari Bathini]

When fadump is enabled, by default 5% of system RAM is reserved for
fadump kernel. While that works for most cases, it is not good enough
for every case.

Currently, to override the default value, fadump supports specifying
memory to reserve with fadump_reserve_mem=size, where only a fixed size
can be specified. This patch adds support to specify memory size to
reserve for different memory ranges as below:

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

Supporting range based input for "fadump_reserve_mem" parameter helps
using the same commandline parameter for different system memory sizes.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
Reviewed-by: Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>
---

Changes from v2:
1. Updated changelog


 arch/powerpc/kernel/fadump.c |   63 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index b3a6633..7c01b5b 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -193,6 +193,55 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
 	return addr;
 }
 
+/*
+ * 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 *name = "fadump_reserve_mem=";
+	char *fadump_cmdline = NULL, *cur;
+
+	fw_dump.reserve_bootvar = 0;
+
+	/* find fadump_reserve_mem and use the last one if there are many */
+	cur = strstr(boot_command_line, name);
+	while (cur) {
+		fadump_cmdline = cur;
+		cur = strstr(cur+1, name);
+	}
+
+	/* when no fadump_reserve_mem= cmdline option is provided */
+	if (!fadump_cmdline)
+		return 0;
+
+	fadump_cmdline += strlen(name);
+
+	/* for fadump_reserve_mem=size cmdline syntax */
+	if (!is_colon_in_param(fadump_cmdline)) {
+		fw_dump.reserve_bootvar = memparse(fadump_cmdline, NULL);
+		return 0;
+	}
+
+	/* for fadump_reserve_mem=ramsize-range:size[,...] cmdline syntax */
+	cur = fadump_cmdline;
+	fw_dump.reserve_bootvar = parse_mem_range_size("fadump_reserve_mem",
+					&cur, memblock_phys_mem_size());
+	if (cur == fadump_cmdline) {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /**
  * fadump_calculate_reserve_size(): reserve variable boot area 5% of System RAM
  *
@@ -212,12 +261,17 @@ 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.
 	 */
 	if (fw_dump.reserve_bootvar)
 		return fw_dump.reserve_bootvar;
+	else
+		printk(KERN_INFO "fadump: calculating default boot size\n");
 
 	/* divide by 20 to get 5% of value */
 	size = memblock_end_of_DRAM() / 20;
@@ -348,15 +402,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;

Re: [PATCH v3 2/2] powerpc/fadump: parse fadump reserve memory size based on memory range

[Dave Young]

On 08/10/16 at 03:35pm, Hari Bathini wrote:
> When fadump is enabled, by default 5% of system RAM is reserved for
> fadump kernel. While that works for most cases, it is not good enough
> for every case.
> 
> Currently, to override the default value, fadump supports specifying
> memory to reserve with fadump_reserve_mem=size, where only a fixed size
> can be specified. This patch adds support to specify memory size to
> reserve for different memory ranges as below:
> 
> 	fadump_reserve_mem=<range1>:<size1>[,<range2>:<size2>,...]

Hi, Hari

I do not understand why you need introduce the new cmdline param, what's
the difference between the "fadump reserved" memory and the memory
reserved by "crashkernel="? Can fadump just use crashkernel= to reserve
memory?

Thanks
Dave

> 
> Supporting range based input for "fadump_reserve_mem" parameter helps
> using the same commandline parameter for different system memory sizes.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> Reviewed-by: Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>
> ---
> 
> Changes from v2:
> 1. Updated changelog
> 
> 
>  arch/powerpc/kernel/fadump.c |   63 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 54 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index b3a6633..7c01b5b 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -193,6 +193,55 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
>  	return addr;
>  }
>  
> +/*
> + * 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 *name = "fadump_reserve_mem=";
> +	char *fadump_cmdline = NULL, *cur;
> +
> +	fw_dump.reserve_bootvar = 0;
> +
> +	/* find fadump_reserve_mem and use the last one if there are many */
> +	cur = strstr(boot_command_line, name);
> +	while (cur) {
> +		fadump_cmdline = cur;
> +		cur = strstr(cur+1, name);
> +	}
> +
> +	/* when no fadump_reserve_mem= cmdline option is provided */
> +	if (!fadump_cmdline)
> +		return 0;
> +
> +	fadump_cmdline += strlen(name);
> +
> +	/* for fadump_reserve_mem=size cmdline syntax */
> +	if (!is_colon_in_param(fadump_cmdline)) {
> +		fw_dump.reserve_bootvar = memparse(fadump_cmdline, NULL);
> +		return 0;
> +	}
> +
> +	/* for fadump_reserve_mem=ramsize-range:size[,...] cmdline syntax */
> +	cur = fadump_cmdline;
> +	fw_dump.reserve_bootvar = parse_mem_range_size("fadump_reserve_mem",
> +					&cur, memblock_phys_mem_size());
> +	if (cur == fadump_cmdline) {
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * fadump_calculate_reserve_size(): reserve variable boot area 5% of System RAM
>   *
> @@ -212,12 +261,17 @@ 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.
>  	 */
>  	if (fw_dump.reserve_bootvar)
>  		return fw_dump.reserve_bootvar;
> +	else
> +		printk(KERN_INFO "fadump: calculating default boot size\n");
>  
>  	/* divide by 20 to get 5% of value */
>  	size = memblock_end_of_DRAM() / 20;
> @@ -348,15 +402,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;
> 

Re: [PATCH v3 2/2] powerpc/fadump: parse fadump reserve memory size based on memory range

[Hari Bathini]

On Thursday 25 August 2016 12:31 PM, Dave Young wrote:
> On 08/10/16 at 03:35pm, Hari Bathini wrote:
>> When fadump is enabled, by default 5% of system RAM is reserved for
>> fadump kernel. While that works for most cases, it is not good enough
>> for every case.
>>
>> Currently, to override the default value, fadump supports specifying
>> memory to reserve with fadump_reserve_mem=size, where only a fixed size
>> can be specified. This patch adds support to specify memory size to
>> reserve for different memory ranges as below:
>>
>> 	fadump_reserve_mem=<range1>:<size1>[,<range2>:<size2>,...]
> Hi, Hari

Hi Dave,

> I do not understand why you need introduce the new cmdline param, what's
> the difference between the "fadump reserved" memory and the memory

I am not introducing a new parameter but adding a new syntax for
an existing parameter.

> reserved by "crashkernel="? Can fadump just use crashkernel= to reserve
> memory?

Not all syntaxes supported by crashkernel apply for fadump_reserve_mem.
Nonetheless, it is worth considering reuse of crashkernel parameter instead
of fadump_reserve_mem. Let me see what I can do about this..

Thanks
Hari

> Thanks
> Dave
>
>> Supporting range based input for "fadump_reserve_mem" parameter helps
>> using the same commandline parameter for different system memory sizes.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
>> Reviewed-by: Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>
>> ---
>>
>> Changes from v2:
>> 1. Updated changelog
>>
>>
>>   arch/powerpc/kernel/fadump.c |   63 ++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 54 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index b3a6633..7c01b5b 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -193,6 +193,55 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
>>   	return addr;
>>   }
>>   
>> +/*
>> + * 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 *name = "fadump_reserve_mem=";
>> +	char *fadump_cmdline = NULL, *cur;
>> +
>> +	fw_dump.reserve_bootvar = 0;
>> +
>> +	/* find fadump_reserve_mem and use the last one if there are many */
>> +	cur = strstr(boot_command_line, name);
>> +	while (cur) {
>> +		fadump_cmdline = cur;
>> +		cur = strstr(cur+1, name);
>> +	}
>> +
>> +	/* when no fadump_reserve_mem= cmdline option is provided */
>> +	if (!fadump_cmdline)
>> +		return 0;
>> +
>> +	fadump_cmdline += strlen(name);
>> +
>> +	/* for fadump_reserve_mem=size cmdline syntax */
>> +	if (!is_colon_in_param(fadump_cmdline)) {
>> +		fw_dump.reserve_bootvar = memparse(fadump_cmdline, NULL);
>> +		return 0;
>> +	}
>> +
>> +	/* for fadump_reserve_mem=ramsize-range:size[,...] cmdline syntax */
>> +	cur = fadump_cmdline;
>> +	fw_dump.reserve_bootvar = parse_mem_range_size("fadump_reserve_mem",
>> +					&cur, memblock_phys_mem_size());
>> +	if (cur == fadump_cmdline) {
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * fadump_calculate_reserve_size(): reserve variable boot area 5% of System RAM
>>    *
>> @@ -212,12 +261,17 @@ 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.
>>   	 */
>>   	if (fw_dump.reserve_bootvar)
>>   		return fw_dump.reserve_bootvar;
>> +	else
>> +		printk(KERN_INFO "fadump: calculating default boot size\n");
>>   
>>   	/* divide by 20 to get 5% of value */
>>   	size = memblock_end_of_DRAM() / 20;
>> @@ -348,15 +402,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;
>>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>

Re: [PATCH v3 2/2] powerpc/fadump: parse fadump reserve memory size based on memory range

[Dave Young]

On 08/25/16 at 11:00pm, Hari Bathini wrote:
> 
> 
> On Thursday 25 August 2016 12:31 PM, Dave Young wrote:
> > On 08/10/16 at 03:35pm, Hari Bathini wrote:
> > > When fadump is enabled, by default 5% of system RAM is reserved for
> > > fadump kernel. While that works for most cases, it is not good enough
> > > for every case.
> > > 
> > > Currently, to override the default value, fadump supports specifying
> > > memory to reserve with fadump_reserve_mem=size, where only a fixed size
> > > can be specified. This patch adds support to specify memory size to
> > > reserve for different memory ranges as below:
> > > 
> > > 	fadump_reserve_mem=<range1>:<size1>[,<range2>:<size2>,...]
> > Hi, Hari
> 
> Hi Dave,
> 
> > I do not understand why you need introduce the new cmdline param, what's
> > the difference between the "fadump reserved" memory and the memory
> 
> I am not introducing a new parameter but adding a new syntax for
> an existing parameter.

Apologize for that, I was not aware it because it is not documented in
kernel-parameters.txt

> 
> > reserved by "crashkernel="? Can fadump just use crashkernel= to reserve
> > memory?
> 
> Not all syntaxes supported by crashkernel apply for fadump_reserve_mem.
> Nonetheless, it is worth considering reuse of crashkernel parameter instead
> of fadump_reserve_mem. Let me see what I can do about this..

Thanks! I originally thought fadump will reserve memory in firmware
code, if it is in kernel then it will be better to just extend and reuse
crashkernel=.

Dave
> 
> Thanks
> Hari
> 
> > Thanks
> > Dave
> > 
> > > Supporting range based input for "fadump_reserve_mem" parameter helps
> > > using the same commandline parameter for different system memory sizes.
> > > 
> > > Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> > > Reviewed-by: Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>
> > > ---
> > > 
> > > Changes from v2:
> > > 1. Updated changelog
> > > 
> > > 
> > >   arch/powerpc/kernel/fadump.c |   63 ++++++++++++++++++++++++++++++++++++------
> > >   1 file changed, 54 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> > > index b3a6633..7c01b5b 100644
> > > --- a/arch/powerpc/kernel/fadump.c
> > > +++ b/arch/powerpc/kernel/fadump.c
> > > @@ -193,6 +193,55 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
> > >   	return addr;
> > >   }
> > > +/*
> > > + * 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 *name = "fadump_reserve_mem=";
> > > +	char *fadump_cmdline = NULL, *cur;
> > > +
> > > +	fw_dump.reserve_bootvar = 0;
> > > +
> > > +	/* find fadump_reserve_mem and use the last one if there are many */
> > > +	cur = strstr(boot_command_line, name);
> > > +	while (cur) {
> > > +		fadump_cmdline = cur;
> > > +		cur = strstr(cur+1, name);
> > > +	}
> > > +
> > > +	/* when no fadump_reserve_mem= cmdline option is provided */
> > > +	if (!fadump_cmdline)
> > > +		return 0;
> > > +
> > > +	fadump_cmdline += strlen(name);
> > > +
> > > +	/* for fadump_reserve_mem=size cmdline syntax */
> > > +	if (!is_colon_in_param(fadump_cmdline)) {
> > > +		fw_dump.reserve_bootvar = memparse(fadump_cmdline, NULL);
> > > +		return 0;
> > > +	}
> > > +
> > > +	/* for fadump_reserve_mem=ramsize-range:size[,...] cmdline syntax */
> > > +	cur = fadump_cmdline;
> > > +	fw_dump.reserve_bootvar = parse_mem_range_size("fadump_reserve_mem",
> > > +					&cur, memblock_phys_mem_size());
> > > +	if (cur == fadump_cmdline) {
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   /**
> > >    * fadump_calculate_reserve_size(): reserve variable boot area 5% of System RAM
> > >    *
> > > @@ -212,12 +261,17 @@ 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.
> > >   	 */
> > >   	if (fw_dump.reserve_bootvar)
> > >   		return fw_dump.reserve_bootvar;
> > > +	else
> > > +		printk(KERN_INFO "fadump: calculating default boot size\n");
> > >   	/* divide by 20 to get 5% of value */
> > >   	size = memblock_end_of_DRAM() / 20;
> > > @@ -348,15 +402,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;
> > > 
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
> > 
>