2017-11-15 15:22:46

by Hari Bathini

[permalink] [raw]
Subject: [PATCH v9 0/8] reduce memory consumption for powerpc firmware-assisted capture kernel

I posted the initial version [1] of patchset [2] adding support to enforce
additional parameters when firmware-assisted dump capture kernel is active.
Michal reposted it with few improvements to parameter processing to make
it more robust. He further posted patchset [3] with few more improvements.

This patch series clubs the work from these two patch-sets while segregating
the generic and arch-specific code to ease the review process.

[1] http://patchwork.ozlabs.org/patch/758176/
[2] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=2733
[3] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=3338

---

Hari Bathini (2):
powerpc/fadump: reduce memory consumption for capture kernel
powerpc/fadump: update documentation about 'fadump_extra_args=' parameter

Michal Suchanek (6):
lib/cmdline.c: remove quotes symmetrically
boot/param: add pointer to current and next argument to unknown parameter callback
lib/cmdline.c: add backslash support to kernel commandline parsing
Documentation/admin-guide: backslash support in commandline
lib/cmdline.c: implement single quotes in commandline argument parsing
Documentation/admin-guide: single quotes in kernel arguments


Documentation/admin-guide/kernel-parameters.rst | 5 +
Documentation/powerpc/firmware-assisted-dump.txt | 20 ++++-
arch/powerpc/include/asm/fadump.h | 2
arch/powerpc/kernel/fadump.c | 97 +++++++++++++++++++++-
arch/powerpc/kernel/prom.c | 7 ++
include/linux/moduleparam.h | 1
init/main.c | 8 +-
kernel/module.c | 5 +
kernel/params.c | 18 +++-
lib/cmdline.c | 54 +++++++-----
lib/dynamic_debug.c | 1
11 files changed, 179 insertions(+), 39 deletions(-)


From 1584145352374174631@xxx Wed Nov 15 15:12:16 +0000 2017
X-GM-THRID: 1584145352374174631
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread


2017-11-15 15:23:19

by Hari Bathini

[permalink] [raw]
Subject: [PATCH v9 3/8] lib/cmdline.c: add backslash support to kernel commandline parsing

From: Michal Suchanek <[email protected]>

This allows passing quotes in kernel arguments. It is useful for passing
nested arguemnts and might be useful if somebody wanted to pass a double
quote directly as part of an argument. It is also useful to have quoting
grammar more similar to shells and bootloaders.

Signed-off-by: Michal Suchanek <[email protected]>
Signed-off-by: Hari Bathini <[email protected]>
---
lib/cmdline.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 6d398a8..d98bdc0 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -193,30 +193,36 @@ bool parse_option_str(const char *str, const char *option)

/*
* Parse a string to get a param value pair.
- * You can use " around spaces, but can't escape ".
+ * You can use " around spaces, and you can escape with \
* Hyphens and underscores equivalent in parameter names.
*/
char *next_arg(char *args, char **param, char **val)
{
unsigned int i, equals = 0;
- int in_quote = 0, quoted = 0;
+ int in_quote = 0, backslash = 0;
char *next;

- if (*args == '"') {
- args++;
- in_quote = 1;
- quoted = 1;
- }
-
for (i = 0; args[i]; i++) {
- if (isspace(args[i]) && !in_quote)
+ if (isspace(args[i]) && !in_quote && !backslash)
break;
- if (equals == 0) {
- if (args[i] == '=')
- equals = i;
+
+ if ((equals == 0) && (args[i] == '='))
+ equals = i;
+
+ if (!backslash) {
+ if ((args[i] == '"') || (args[i] == '\\')) {
+ if (args[i] == '"')
+ in_quote = !in_quote;
+ if (args[i] == '\\')
+ backslash = 1;
+
+ memmove(args + 1, args, i);
+ args++;
+ i--;
+ }
+ } else {
+ backslash = 0;
}
- if (args[i] == '"')
- in_quote = !in_quote;
}

*param = args;
@@ -225,13 +231,6 @@ char *next_arg(char *args, char **param, char **val)
else {
args[equals] = '\0';
*val = args + equals + 1;
-
- /* Don't include quotes in value. */
- if ((args[i-1] == '"') && ((quoted) || (**val == '"'))) {
- args[i-1] = '\0';
- if (!quoted)
- (*val)++;
- }
}

if (args[i]) {


From 1584150090996393690@xxx Wed Nov 15 16:27:35 +0000 2017
X-GM-THRID: 1584150090996393690
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-15 15:26:52

by Hari Bathini

[permalink] [raw]
Subject: [PATCH v9 8/8] powerpc/fadump: update documentation about 'fadump_extra_args=' parameter

With the introduction of 'fadump_extra_args=' parameter to pass additional
parameters to fadump (capture) kernel, update documentation about it.

Signed-off-by: Hari Bathini <[email protected]>
Signed-off-by: Michal Suchanek <[email protected]>
---
Documentation/powerpc/firmware-assisted-dump.txt | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/powerpc/firmware-assisted-dump.txt b/Documentation/powerpc/firmware-assisted-dump.txt
index bdd344a..5705f55 100644
--- a/Documentation/powerpc/firmware-assisted-dump.txt
+++ b/Documentation/powerpc/firmware-assisted-dump.txt
@@ -162,7 +162,19 @@ How to enable firmware-assisted dump (fadump):

1. Set config option CONFIG_FA_DUMP=y and build kernel.
2. Boot into linux kernel with 'fadump=on' kernel cmdline option.
-3. Optionally, user can also set 'crashkernel=' kernel cmdline
+3. A user can pass additional command line parameters as a space
+ separated quoted list through 'fadump_extra_args=' parameter,
+ to be enforced when fadump is active. For example, parameter
+ 'fadump_extra_args="nr_cpus=1 numa=off udev.children-max=2"'
+ will be changed to 'fadump_extra_args nr_cpus=1 numa=off
+ udev.children-max=2' in-place when fadump is active. This
+ parameter has no affect when fadump is not active. Multiple
+ instances of 'fadump_extra_args=' can be passed. This provision
+ can be used to reduce memory consumption during dump capture by
+ disabling unwarranted resources/subsystems like CPUs, NUMA
+ and such. Value with spaces can be passed as
+ 'fadump_extra_args="parameter=\"value with spaces\""'
+4. Optionally, user can also set 'crashkernel=' kernel cmdline
to specify size of the memory to reserve for boot memory dump
preservation.

@@ -172,6 +184,12 @@ NOTE: 1. 'fadump_reserve_mem=' parameter has been deprecated. Instead
2. If firmware-assisted dump fails to reserve memory then it
will fallback to existing kdump mechanism if 'crashkernel='
option is set at kernel cmdline.
+ 3. Special parameters like '--' passed inside fadump_extra_args are also
+ just left in-place. So, the user is advised to consider this while
+ specifying such parameters. It may be required to quote the argument
+ to fadump_extra_args when the bootloader uses double-quotes as
+ argument delimiter as well. eg
+ append = " fadump_extra_args=\"nr_cpus=1 numa=off udev.children-max=2\""

Sysfs/debugfs files:
------------


From 1584128024182816601@xxx Wed Nov 15 10:36:50 +0000 2017
X-GM-THRID: 1583985575029515476
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-15 15:23:14

by Hari Bathini

[permalink] [raw]
Subject: [PATCH v9 2/8] boot/param: add pointer to current and next argument to unknown parameter callback

From: Michal Suchanek <[email protected]>

Add pointer to current and next argument to make parameter processing
more robust. This can make parameter processing easier and less error
prone in cases where the parameters need to be enforced/ignored based
on firmware/system state.

Signed-off-by: Michal Suchanek <[email protected]>
Signed-off-by: Hari Bathini <[email protected]>
---

Changes in v9:
* Fixed messages like below observed while loading modules with no parameters.
- iptable_filter: unknown parameter '' ignored
- ip_tables: unknown parameter '' ignored


include/linux/moduleparam.h | 1 +
init/main.c | 8 ++++++--
kernel/module.c | 5 +++--
kernel/params.c | 18 ++++++++++++------
lib/dynamic_debug.c | 1 +
5 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 1d7140f..50a19e6 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -328,6 +328,7 @@ extern char *parse_args(const char *name,
s16 level_max,
void *arg,
int (*unknown)(char *param, char *val,
+ char *currant, char *next,
const char *doing, void *arg));

/* Called by module remove. */
diff --git a/init/main.c b/init/main.c
index 3bdd8da..3ba3eed 100644
--- a/init/main.c
+++ b/init/main.c
@@ -241,6 +241,7 @@ early_param("loglevel", loglevel);

/* Change NUL term back to "=", to make "param" the whole string. */
static int __init repair_env_string(char *param, char *val,
+ char *unused3, char *unused2,
const char *unused, void *arg)
{
if (val) {
@@ -259,6 +260,7 @@ static int __init repair_env_string(char *param, char *val,

/* Anything after -- gets handed straight to init. */
static int __init set_init_arg(char *param, char *val,
+ char *unused3, char *unused2,
const char *unused, void *arg)
{
unsigned int i;
@@ -266,7 +268,7 @@ static int __init set_init_arg(char *param, char *val,
if (panic_later)
return 0;

- repair_env_string(param, val, unused, NULL);
+ repair_env_string(param, val, unused3, unused2, unused, NULL);

for (i = 0; argv_init[i]; i++) {
if (i == MAX_INIT_ARGS) {
@@ -284,9 +286,10 @@ static int __init set_init_arg(char *param, char *val,
* unused parameters (modprobe will find them in /proc/cmdline).
*/
static int __init unknown_bootoption(char *param, char *val,
+ char *unused3, char *unused2,
const char *unused, void *arg)
{
- repair_env_string(param, val, unused, NULL);
+ repair_env_string(param, val, unused3, unused2, unused, NULL);

/* Handle obsolete-style parameters */
if (obsolete_checksetup(param))
@@ -438,6 +441,7 @@ static noinline void __ref rest_init(void)

/* Check for early params. */
static int __init do_early_param(char *param, char *val,
+ char *unused3, char *unused2,
const char *unused, void *arg)
{
const struct obs_kernel_param *p;
diff --git a/kernel/module.c b/kernel/module.c
index 32c2cda..ffe7520 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3619,8 +3619,9 @@ static int prepare_coming_module(struct module *mod)
return 0;
}

-static int unknown_module_param_cb(char *param, char *val, const char *modname,
- void *arg)
+static int unknown_module_param_cb(char *param, char *val,
+ char *unused, char *unused2,
+ const char *modname, void *arg)
{
struct module *mod = arg;
int ret;
diff --git a/kernel/params.c b/kernel/params.c
index cc9108c..69ff58e 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -119,6 +119,8 @@ static void param_check_unsafe(const struct kernel_param *kp)

static int parse_one(char *param,
char *val,
+ char *currant,
+ char *next,
const char *doing,
const struct kernel_param *params,
unsigned num_params,
@@ -126,7 +128,8 @@ static int parse_one(char *param,
s16 max_level,
void *arg,
int (*handle_unknown)(char *param, char *val,
- const char *doing, void *arg))
+ char *currant, char *next,
+ const char *doing, void *arg))
{
unsigned int i;
int err;
@@ -153,7 +156,7 @@ static int parse_one(char *param,

if (handle_unknown) {
pr_debug("doing %s: %s='%s'\n", doing, param, val);
- return handle_unknown(param, val, doing, arg);
+ return handle_unknown(param, val, currant, next, doing, arg);
}

pr_debug("Unknown argument '%s'\n", param);
@@ -169,9 +172,10 @@ char *parse_args(const char *doing,
s16 max_level,
void *arg,
int (*unknown)(char *param, char *val,
+ char *currant, char *next,
const char *doing, void *arg))
{
- char *param, *val, *err = NULL;
+ char *param, *val, *next, *err = NULL;

/* Chew leading spaces */
args = skip_spaces(args);
@@ -179,16 +183,18 @@ char *parse_args(const char *doing,
if (*args)
pr_debug("doing %s, parsing ARGS: '%s'\n", doing, args);

- while (*args) {
+ next = next_arg(args, &param, &val);
+ while (*next) {
int ret;
int irq_was_disabled;

- args = next_arg(args, &param, &val);
+ args = next;
+ next = next_arg(args, &param, &val);
/* Stop at -- */
if (!val && strcmp(param, "--") == 0)
return err ?: args;
irq_was_disabled = irqs_disabled();
- ret = parse_one(param, val, doing, params, num,
+ ret = parse_one(param, val, args, next, doing, params, num,
min_level, max_level, arg, unknown);
if (irq_was_disabled && !irqs_disabled())
pr_warn("%s: option '%s' enabled irq's!\n",
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index da796e2..dec7f40 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -889,6 +889,7 @@ static int ddebug_dyndbg_param_cb(char *param, char *val,

/* handle both dyndbg and $module.dyndbg params at boot */
static int ddebug_dyndbg_boot_param_cb(char *param, char *val,
+ char *unused3, char *unused2,
const char *unused, void *arg)
{
vpr_info("%s=\"%s\"\n", param, val);


From 1584180761991488189@xxx Thu Nov 16 00:35:05 +0000 2017
X-GM-THRID: 1584180761991488189
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-15 15:22:56

by Hari Bathini

[permalink] [raw]
Subject: [PATCH v9 1/8] lib/cmdline.c: remove quotes symmetrically

From: Michal Suchanek <[email protected]>

Remove quotes from argument value only if there is qoute on both sides.

Signed-off-by: Michal Suchanek <[email protected]>
---
lib/cmdline.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 171c19b..6d398a8 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -227,14 +227,12 @@ char *next_arg(char *args, char **param, char **val)
*val = args + equals + 1;

/* Don't include quotes in value. */
- if (**val == '"') {
- (*val)++;
- if (args[i-1] == '"')
- args[i-1] = '\0';
+ if ((args[i-1] == '"') && ((quoted) || (**val == '"'))) {
+ args[i-1] = '\0';
+ if (!quoted)
+ (*val)++;
}
}
- if (quoted && args[i-1] == '"')
- args[i-1] = '\0';

if (args[i]) {
args[i] = '\0';


From 1584100086110193330@xxx Wed Nov 15 03:12:46 +0000 2017
X-GM-THRID: 1584098729596927351
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-15 16:34:25

by Hari Bathini

[permalink] [raw]
Subject: [PATCH v9 6/8] Documentation/admin-guide: single quotes in kernel arguments

From: Michal Suchanek <[email protected]>

Signed-off-by: Michal Suchanek <[email protected]>
---
Documentation/admin-guide/kernel-parameters.rst | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index 722d3f7..1f98372 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -35,9 +35,10 @@ can also be entered as::

log-buf-len=1M print_fatal_signals=1

-Double-quotes and backslashes can be used to protect spaces in values, e.g.::
+Double-quotes single-quotes and backslashes can be used to protect spaces
+in values, e.g.::

- param="spaces in here" param2=spaces\ in\ here
+ param="spaces in here" param2=spaces\ in\ here param3='@%# !\'

cpu lists:
----------


From 1584006890750509205@xxx Tue Nov 14 02:31:28 +0000 2017
X-GM-THRID: 1583741173631733006
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-15 16:59:43

by Hari Bathini

[permalink] [raw]
Subject: [PATCH v9 5/8] lib/cmdline.c: implement single quotes in commandline argument parsing

From: Michal Suchanek <[email protected]>

This brings the kernel parser about on par with bourne shell, grub, and
other tools that chew the arguments before kernel does.

This should make it easier to deal with multiple levels of
nesting/quoting. With same quoting grammar on each level there is less
room for confusion.

Signed-off-by: Michal Suchanek <[email protected]>
---
lib/cmdline.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index d98bdc0..c5335a7 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -191,34 +191,45 @@ bool parse_option_str(const char *str, const char *option)
return false;
}

+#define squash_char { \
+ memmove(args + 1, args, i); \
+ args++; \
+ i--; \
+}
+
/*
* Parse a string to get a param value pair.
- * You can use " around spaces, and you can escape with \
+ * You can use " or ' around spaces, and you can escape with \
* Hyphens and underscores equivalent in parameter names.
*/
char *next_arg(char *args, char **param, char **val)
{
unsigned int i, equals = 0;
- int in_quote = 0, backslash = 0;
+ int in_quote = 0, backslash = 0, in_single = 0;
char *next;

for (i = 0; args[i]; i++) {
- if (isspace(args[i]) && !in_quote && !backslash)
+ if (isspace(args[i]) && !in_quote && !backslash && !in_single)
break;

if ((equals == 0) && (args[i] == '='))
equals = i;

- if (!backslash) {
- if ((args[i] == '"') || (args[i] == '\\')) {
+ if (in_single) {
+ if (args[i] == '\'') {
+ in_single = 0;
+ squash_char;
+ }
+ } else if (!backslash) {
+ if ((args[i] == '"') || (args[i] == '\\') ||
+ (args[i] == '\'')) {
if (args[i] == '"')
in_quote = !in_quote;
if (args[i] == '\\')
backslash = 1;
-
- memmove(args + 1, args, i);
- args++;
- i--;
+ if (args[i] == '\'')
+ in_single = 1;
+ squash_char;
}
} else {
backslash = 0;


From 1583482865510504420@xxx Wed Nov 08 07:42:19 +0000 2017
X-GM-THRID: 1582925127753096862
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-15 15:23:26

by Hari Bathini

[permalink] [raw]
Subject: [PATCH v9 4/8] Documentation/admin-guide: backslash support in commandline

From: Michal Suchanek <[email protected]>

Signed-off-by: Michal Suchanek <[email protected]>
---
Documentation/admin-guide/kernel-parameters.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index b2598cc..722d3f7 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -35,9 +35,9 @@ can also be entered as::

log-buf-len=1M print_fatal_signals=1

-Double-quotes can be used to protect spaces in values, e.g.::
+Double-quotes and backslashes can be used to protect spaces in values, e.g.::

- param="spaces in here"
+ param="spaces in here" param2=spaces\ in\ here

cpu lists:
----------


From 1585517337770158004@xxx Thu Nov 30 18:39:23 +0000 2017
X-GM-THRID: 1585480200704436794
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread