On 07/05/2016 10:48 AM, Michael Ellerman wrote:
On 06/24/2016 10:56 AM, Michael Ellerman wrote:
On Wed, 2016-22-06 at 19:25:26 UTC, Hari Bathini wrote:
...
While the code is moved to kernel/params.c file, there is no change in logic
for crashkernel parameter parsing as the moved code is invoked with function
calls at appropriate places.
Hi Michael,
Are you sure that's true?
Yes. I tested it.
The old code would return -EINVAL from parse_crashkernel_mem() for any
error, regardless of whether it had already parsed some of the string.
eg:
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 56b3ed0..d43f5cc 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1083,59 +1083,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;
- }
So eg, if I give it "128M-foo" it will modify cur, and then error out here ^
It does modify cur (local variable) but that would have no bearing on
parsing logic
as we are returning immediately..
You've changed that to:
+ *crash_size = parse_mem_range_size("crashkernel", &cur, system_ram);
+ if (cur == cmdline)
+ return -EINVAL;
Which only returns EINVAL if cur is not modified at all.
I think the confusion is with the same local variable cur in
parse_crashkernel_mem()
& parse_mem_range_size() functions.
We modified cur (local variable) in parse_mem_range_size() but the
output parameter (char **str)
remains unchanged unless we find a match.
Thanks
Hari
And looking below:
diff --git a/kernel/params.c b/kernel/params.c
index a6d6149..84e40ae 100644
--- a/kernel/params.c
+++ b/kernel/params.c
...
+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;
If we error out here for example, we have modified cur, so the code above
*won't* return EINVAL.
Which looks like a behaviour change to me?
cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
