* [PATCH 1/2] powerpc/fadump: return 0 on re-registration @ 2017-06-26 14:06 Michal Suchanek 2017-06-26 14:06 ` [PATCH 2/2] powerpc/fadump: use kstrtoint to handle sysfs store Michal Suchanek 2017-06-27 9:30 ` [PATCH 1/2] powerpc/fadump: return 0 on re-registration Michal Suchánek 0 siblings, 2 replies; 5+ messages in thread From: Michal Suchanek @ 2017-06-26 14:06 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Hari Bathini, Mahesh Salgaonkar, Andrew Morton, Michal Suchanek, Colin Ian King, linuxppc-dev, linux-kernel When fadump is already registered return success. Currently EEXIST is returned which is difficult to handle race-free in userspace when shell scripts are used. If multiple writers are trying to write '1' there is no difference in whichever succeeds so just return 0 to all. Signed-off-by: Michal Suchanek <msuchanek@suse.de> --- arch/powerpc/kernel/fadump.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index 436aedf195ab..5a7355381dac 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -1214,7 +1214,6 @@ static ssize_t fadump_register_store(struct kobject *kobj, break; case '1': if (fw_dump.dump_registered == 1) { - ret = -EEXIST; goto unlock_out; } /* Register Firmware-assisted dump */ -- 2.10.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] powerpc/fadump: use kstrtoint to handle sysfs store 2017-06-26 14:06 [PATCH 1/2] powerpc/fadump: return 0 on re-registration Michal Suchanek @ 2017-06-26 14:06 ` Michal Suchanek 2017-11-12 17:30 ` Hari Bathini 2017-11-14 11:11 ` [2/2] " Michael Ellerman 2017-06-27 9:30 ` [PATCH 1/2] powerpc/fadump: return 0 on re-registration Michal Suchánek 1 sibling, 2 replies; 5+ messages in thread From: Michal Suchanek @ 2017-06-26 14:06 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Hari Bathini, Mahesh Salgaonkar, Andrew Morton, Michal Suchanek, Colin Ian King, linuxppc-dev, linux-kernel Currently sysfs store handlers in fadump use if buf[0] == 'char'. This means input "100foo" is interpreted as '1' and "01" as '0'. Change to kstrtoint so leading zeroes and the like is handled in expected way. Signed-off-by: Michal Suchanek <msuchanek@suse.de> --- arch/powerpc/kernel/fadump.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index 5a7355381dac..241eff0b5f76 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -1161,10 +1161,15 @@ static ssize_t fadump_release_memory_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count) { + int input = -1; + if (!fw_dump.dump_active) return -EPERM; - if (buf[0] == '1') { + if (kstrtoint(buf, 0, &input)) + return -EINVAL; + + if (input == 1) { /* * Take away the '/proc/vmcore'. We are releasing the dump * memory, hence it will not be valid anymore. @@ -1198,21 +1203,25 @@ static ssize_t fadump_register_store(struct kobject *kobj, const char *buf, size_t count) { int ret = 0; + int input = -1; if (!fw_dump.fadump_enabled || fdm_active) return -EPERM; + if (kstrtoint(buf, 0, &input)) + return -EINVAL; + mutex_lock(&fadump_mutex); - switch (buf[0]) { - case '0': + switch (input) { + case 0: if (fw_dump.dump_registered == 0) { goto unlock_out; } /* Un-register Firmware-assisted dump */ fadump_unregister_dump(&fdm); break; - case '1': + case 1: if (fw_dump.dump_registered == 1) { goto unlock_out; } -- 2.10.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] powerpc/fadump: use kstrtoint to handle sysfs store 2017-06-26 14:06 ` [PATCH 2/2] powerpc/fadump: use kstrtoint to handle sysfs store Michal Suchanek @ 2017-11-12 17:30 ` Hari Bathini 2017-11-14 11:11 ` [2/2] " Michael Ellerman 1 sibling, 0 replies; 5+ messages in thread From: Hari Bathini @ 2017-11-12 17:30 UTC (permalink / raw) To: Michal Suchanek, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Mahesh Salgaonkar, Andrew Morton, Colin Ian King, linuxppc-dev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1892 bytes --] Thanks for the patch, Michal. On Monday 26 June 2017 07:36 PM, Michal Suchanek wrote: > Currently sysfs store handlers in fadump use if buf[0] == 'char'. > > This means input "100foo" is interpreted as '1' and "01" as '0'. > > Change to kstrtoint so leading zeroes and the like is handled in > expected way. > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> Acked-by: Hari Bathini <hbathini@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/fadump.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c > index 5a7355381dac..241eff0b5f76 100644 > --- a/arch/powerpc/kernel/fadump.c > +++ b/arch/powerpc/kernel/fadump.c > @@ -1161,10 +1161,15 @@ static ssize_t fadump_release_memory_store(struct kobject *kobj, > struct kobj_attribute *attr, > const char *buf, size_t count) > { > + int input = -1; > + > if (!fw_dump.dump_active) > return -EPERM; > > - if (buf[0] == '1') { > + if (kstrtoint(buf, 0, &input)) > + return -EINVAL; > + > + if (input == 1) { > /* > * Take away the '/proc/vmcore'. We are releasing the dump > * memory, hence it will not be valid anymore. > @@ -1198,21 +1203,25 @@ static ssize_t fadump_register_store(struct kobject *kobj, > const char *buf, size_t count) > { > int ret = 0; > + int input = -1; > > if (!fw_dump.fadump_enabled || fdm_active) > return -EPERM; > > + if (kstrtoint(buf, 0, &input)) > + return -EINVAL; > + > mutex_lock(&fadump_mutex); > > - switch (buf[0]) { > - case '0': > + switch (input) { > + case 0: > if (fw_dump.dump_registered == 0) { > goto unlock_out; > } > /* Un-register Firmware-assisted dump */ > fadump_unregister_dump(&fdm); > break; > - case '1': > + case 1: > if (fw_dump.dump_registered == 1) { > goto unlock_out; > } [-- Attachment #2: Type: text/html, Size: 2472 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [2/2] powerpc/fadump: use kstrtoint to handle sysfs store 2017-06-26 14:06 ` [PATCH 2/2] powerpc/fadump: use kstrtoint to handle sysfs store Michal Suchanek 2017-11-12 17:30 ` Hari Bathini @ 2017-11-14 11:11 ` Michael Ellerman 1 sibling, 0 replies; 5+ messages in thread From: Michael Ellerman @ 2017-11-14 11:11 UTC (permalink / raw) To: Michal Suchanek, Benjamin Herrenschmidt, Paul Mackerras, Hari Bathini, Mahesh Salgaonkar, Andrew Morton, Michal Suchanek, Colin Ian King, linuxppc-dev, linux-kernel On Mon, 2017-06-26 at 14:06:01 UTC, Michal Suchanek wrote: > Currently sysfs store handlers in fadump use if buf[0] == 'char'. > > This means input "100foo" is interpreted as '1' and "01" as '0'. > > Change to kstrtoint so leading zeroes and the like is handled in > expected way. > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > Acked-by: Hari Bathini <hbathini@linux.vnet.ibm.com> > Signed-off-by: Michal Suchanek <a class="moz-txt-link-rfc2396E" href="mailto:msuchanek@suse.de"><msuchanek@suse.de></a></pre> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/dcdc46794b7bb76733d9792cca2f45 cheers ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] powerpc/fadump: return 0 on re-registration 2017-06-26 14:06 [PATCH 1/2] powerpc/fadump: return 0 on re-registration Michal Suchanek 2017-06-26 14:06 ` [PATCH 2/2] powerpc/fadump: use kstrtoint to handle sysfs store Michal Suchanek @ 2017-06-27 9:30 ` Michal Suchánek 1 sibling, 0 replies; 5+ messages in thread From: Michal Suchánek @ 2017-06-27 9:30 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Hari Bathini, Mahesh Salgaonkar, Andrew Morton, Michal Suchanek, Colin Ian King, linuxppc-dev, linux-kernel On Mon, 26 Jun 2017 16:06:00 +0200 Michal Suchanek <msuchanek@suse.de> wrote: > When fadump is already registered return success. > > Currently EEXIST is returned which is difficult to handle race-free in > userspace when shell scripts are used. If multiple writers are trying > to write '1' there is no difference in whichever succeeds so just > return 0 to all. > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > --- > arch/powerpc/kernel/fadump.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/powerpc/kernel/fadump.c > b/arch/powerpc/kernel/fadump.c index 436aedf195ab..5a7355381dac 100644 > --- a/arch/powerpc/kernel/fadump.c > +++ b/arch/powerpc/kernel/fadump.c > @@ -1214,7 +1214,6 @@ static ssize_t fadump_register_store(struct > kobject *kobj, break; > case '1': > if (fw_dump.dump_registered == 1) { > - ret = -EEXIST; > goto unlock_out; > } > /* Register Firmware-assisted dump */ Forget about this one. It breaks another case when fadump is registered and you need to re-register to account for change in system configuration. Thanks Michal ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-14 11:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-26 14:06 [PATCH 1/2] powerpc/fadump: return 0 on re-registration Michal Suchanek 2017-06-26 14:06 ` [PATCH 2/2] powerpc/fadump: use kstrtoint to handle sysfs store Michal Suchanek 2017-11-12 17:30 ` Hari Bathini 2017-11-14 11:11 ` [2/2] " Michael Ellerman 2017-06-27 9:30 ` [PATCH 1/2] powerpc/fadump: return 0 on re-registration Michal Suchánek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).