Re: [PATCH] i2c: don't print error when adding adapter fails
t; 0)
> goto err_clk_notifier;
> - }
>
> dev_info(>dev, "Initialized RK3xxx I2C bus at %p\n", i2c->regs);
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c
> b/drivers/i2c/busses/i2c-s3c2410.c
> index 38dc1cacfd8bba..499af26e736e7e 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -1215,7 +1215,6 @@ static int s3c24xx_i2c_probe(struct platform_device
> *pdev)
>
> ret = i2c_add_numbered_adapter(>adap);
> if (ret < 0) {
> - dev_err(>dev, "failed to add bus to i2c core\n");
> pm_runtime_disable(>dev);
> s3c24xx_i2c_deregister_cpufreq(i2c);
> clk_unprepare(i2c->clk);
> diff --git a/drivers/i2c/busses/i2c-sh7760.c b/drivers/i2c/busses/i2c-sh7760.c
> index 24968384b4014f..c2005c789d2b09 100644
> --- a/drivers/i2c/busses/i2c-sh7760.c
> +++ b/drivers/i2c/busses/i2c-sh7760.c
> @@ -510,10 +510,8 @@ static int sh7760_i2c_probe(struct platform_device *pdev)
> }
>
> ret = i2c_add_numbered_adapter(>adap);
> - if (ret < 0) {
> - dev_err(>dev, "reg adap failed: %d\n", ret);
> + if (ret < 0)
> goto out4;
> - }
>
> platform_set_drvdata(pdev, id);
>
> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c
> b/drivers/i2c/busses/i2c-sh_mobile.c
> index 6fb3e264599229..50f276e8d17967 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -981,7 +981,6 @@ static int sh_mobile_i2c_probe(struct platform_device
> *dev)
> ret = i2c_add_numbered_adapter(adap);
> if (ret < 0) {
> sh_mobile_i2c_release_dma(pd);
> - dev_err(>dev, "cannot add numbered adapter\n");
> return ret;
> }
>
> diff --git a/drivers/i2c/busses/i2c-sirf.c b/drivers/i2c/busses/i2c-sirf.c
> index 792a42bdd335b9..95e81d0f72b4d2 100644
> --- a/drivers/i2c/busses/i2c-sirf.c
> +++ b/drivers/i2c/busses/i2c-sirf.c
> @@ -387,10 +387,8 @@ static int i2c_sirfsoc_probe(struct platform_device
> *pdev)
> writel(regval, siic->base + SIRFSOC_I2C_SDA_DELAY);
>
> err = i2c_add_numbered_adapter(adap);
> - if (err < 0) {
> - dev_err(>dev, "Can't add new i2c adapter\n");
> + if (err < 0)
> goto out;
> - }
>
> clk_disable(clk);
>
> diff --git a/drivers/i2c/busses/i2c-st.c b/drivers/i2c/busses/i2c-st.c
> index 944ec420508487..1371547ce1a3a8 100644
> --- a/drivers/i2c/busses/i2c-st.c
> +++ b/drivers/i2c/busses/i2c-st.c
> @@ -874,10 +874,8 @@ static int st_i2c_probe(struct platform_device *pdev)
> init_completion(_dev->complete);
>
> ret = i2c_add_adapter(adap);
> - if (ret) {
> - dev_err(>dev, "Failed to add adapter\n");
> + if (ret)
> return ret;
> - }
>
> platform_set_drvdata(pdev, i2c_dev);
>
> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> index 460c134832ac30..dc63236b45b216 100644
> --- a/drivers/i2c/busses/i2c-stu300.c
> +++ b/drivers/i2c/busses/i2c-stu300.c
> @@ -920,11 +920,8 @@ static int stu300_probe(struct platform_device *pdev)
>
> /* i2c device drivers may be active on return from add_adapter() */
> ret = i2c_add_numbered_adapter(adap);
> - if (ret) {
> - dev_err(>dev, "failure adding ST Micro DDC "
> -"I2C adapter\n");
> + if (ret)
> return ret;
> - }
>
> platform_set_drvdata(pdev, dev);
> dev_info(>dev, "ST DDC I2C @ %p, irq %d\n",
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index b126dbaa47e370..d9979da11485ae 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -932,10 +932,8 @@ static int tegra_i2c_probe(struct platform_device *pdev)
> i2c_dev->adapter.dev.of_node = pdev->dev.of_node;
>
> ret = i2c_add_numbered_adapter(_dev->adapter);
> - if (ret) {
> - dev_err(>dev, "Failed to add I2C adapter\n");
> + if (ret)
> goto disable_div_clk;
> - }
>
> return 0;
>
> diff --git a/drivers/i2c/busses/i2c-uniphier-f.c
> b/drivers/i2c/busses/i2c-uniphier-f.c
> index aeead0d27d1007..64318e69089439 100644
> --- a/drivers/i2c/busses/i2c-uniphier-f.c
> +++ b/drivers/i2c/busses/i2c-uniphier-f.c
> @@ -550,15 +550,10 @@ static int uniphier_fi2c_probe(struct platform_device
> *pdev)
> }
>
> ret = i2c_add_adapter(>adap);
> - if (ret) {
> - dev_err(dev, "failed to add I2C adapter\n");
> - goto err;
> - }
> -
> -err:
> if (ret)
> clk_disable_unprepare(priv->clk);
>
> + err:
> return ret;
> }
>
> diff --git a/drivers/i2c/busses/i2c-uniphier.c
> b/drivers/i2c/busses/i2c-uniphier.c
> index 475a5eb514e215..94f64cccfdef08 100644
> --- a/drivers/i2c/busses/i2c-uniphier.c
> +++ b/drivers/i2c/busses/i2c-uniphier.c
> @@ -407,15 +407,10 @@ static int uniphier_i2c_probe(struct platform_device
> *pdev)
> }
>
> ret = i2c_add_adapter(>adap);
> - if (ret) {
> - dev_err(dev, "failed to add I2C adapter\n");
> - goto err;
> - }
> -
> -err:
> if (ret)
> clk_disable_unprepare(priv->clk);
>
> + err:
> return ret;
> }
>
> diff --git a/drivers/i2c/busses/i2c-wmt.c b/drivers/i2c/busses/i2c-wmt.c
> index e1e3a85596c562..fbd0fd59f31239 100644
> --- a/drivers/i2c/busses/i2c-wmt.c
> +++ b/drivers/i2c/busses/i2c-wmt.c
> @@ -432,10 +432,8 @@ static int wmt_i2c_probe(struct platform_device *pdev)
> }
>
> err = i2c_add_adapter(adap);
> - if (err) {
> - dev_err(>dev, "failed to add adapter\n");
> + if (err)
> return err;
> - }
>
> platform_set_drvdata(pdev, i2c_dev);
>
> diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c
> b/drivers/i2c/busses/i2c-xgene-slimpro.c
> index 4233f5695352fd..263685c7a51287 100644
> --- a/drivers/i2c/busses/i2c-xgene-slimpro.c
> +++ b/drivers/i2c/busses/i2c-xgene-slimpro.c
> @@ -418,7 +418,6 @@ static int xgene_slimpro_i2c_probe(struct platform_device
> *pdev)
> i2c_set_adapdata(adapter, ctx);
> rc = i2c_add_adapter(adapter);
> if (rc) {
> - dev_err(>dev, "Adapter registeration failed\n");
> mbox_free_channel(ctx->mbox_chan);
> return rc;
> }
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index 74f54f2f471fa7..66bce3b311a199 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -804,7 +804,6 @@ static int xiic_i2c_probe(struct platform_device *pdev)
> /* add i2c adapter to i2c tree */
> ret = i2c_add_adapter(>adap);
> if (ret) {
> - dev_err(>dev, "Failed to add adapter\n");
> xiic_deinit(i2c);
> goto err_clk_dis;
> }
> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> index 55a7bef1b2e1df..2a972ed7aa0df1 100644
> --- a/drivers/i2c/busses/i2c-xlp9xx.c
> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
> @@ -400,10 +400,8 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
> i2c_set_adapdata(>adapter, priv);
>
> err = i2c_add_adapter(>adapter);
> - if (err) {
> - dev_err(>dev, "failed to add I2C adapter!\n");
> + if (err)
> return err;
> - }
>
> platform_set_drvdata(pdev, priv);
> dev_dbg(>dev, "I2C bus:%d added\n", priv->adapter.nr);
> diff --git a/drivers/i2c/busses/i2c-xlr.c b/drivers/i2c/busses/i2c-xlr.c
> index 613c3a4f2c5142..0968f59b6df586 100644
> --- a/drivers/i2c/busses/i2c-xlr.c
> +++ b/drivers/i2c/busses/i2c-xlr.c
> @@ -432,10 +432,8 @@ static int xlr_i2c_probe(struct platform_device *pdev)
>
> i2c_set_adapdata(>adap, priv);
> ret = i2c_add_numbered_adapter(>adap);
> - if (ret < 0) {
> - dev_err(>adap.dev, "Failed to add i2c bus.\n");
> + if (ret < 0)
> return ret;
> - }
>
> platform_set_drvdata(pdev, priv);
> dev_info(>adap.dev, "Added I2C Bus.\n");
> --
> 2.8.1
>
>
Acked-by: Neil Horman <nhor...@tuxdriver.com>
Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
On Tue, Apr 08, 2014 at 09:56:10AM +0200, Gerhard Sittig wrote: [ removed cscope-devel from Cc:, non-subscriber mails get blocked anyway ] On Mon, 2014-04-07 at 14:42 +0200, Gerhard Sittig wrote: On Mon, 2014-04-07 at 06:42 -0400, Neil Horman wrote: On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote: [ ... ] cscope reports error when generating the cross-reference database: $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope GEN cscope cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S And when calling cscope from ./obj-cscope/ directory, it reports errors too. Hopefully it doesn't stop it from working, so I'm still able to use cscope to browse kernel sources. No, it won't stop it from working, it just won't search those files. I don't recall exactly the reason, but IIRC there was a big discussion long ago about symlinks and our ability to support them (around version 1.94 I think). We decided to not handle symlinks, as they would either point outside our search tree, which we didn't want to include, or would point to another file in the search tree, which made loading them pointless (as we would cover the search in the pointed file). So there are valid reasons to not process those filesystem entries. Would it be useful to not emit the warnings then? Or to silent those warnings when the user knows it's perfectly legal to skip those filesytem entries? Like what you can do with the ctags(1) command and its --links option. For the record: I got a response off list (actually to the cscope list only which is closed for non-subscribers, while the Linux related recipients were removed despite the fact that the issue appears to be in Linux), see http://article.gmane.org/gmane.comp.programming.tools.cscope.devel/105 I don't agree with Hans here. He's right in that the Linux makefiles could exclude the symlinks from the index file that it builds, but if cscope were left to its own devices it would also exclude the assembly files and other code that it doesn't officially parse, but does more or less ok with. We can argue all day weather its ok for Linux to do that, but the facts of the matter is that people use cscope to search the linux source tree asm files and all, and while we could propose that the cscope makefile target filter out symlinks, it seems IMHO to be more difficult than its worth. Adding code to silence db build warnings would be good. This reponse suggests that the issue is not in cscope(1) itself, but instead is in how the cscope(1) command got invoked. Which translates into the Linux infrastructure does something wrong. A quick search identifies ./scripts/tags.sh:all_target_sources() as the spot where symlinks should get filtered out. Where both paths of all_target_sources() end up calling all_sources(). virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote: Hi, I'm using cscope to browse kernel sources, but I'm facing warnings from the tool since following commit: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=22d651dcef536c75f75537290bf3da5038e68b6b commit 22d651dcef536c75f75537290bf3da5038e68b6b Author: Michael Ellerman m...@ellerman.id.au Date: Tue Jan 21 15:22:17 2014 +1100 selftests/powerpc: Import Anton's memcpy / copy_tofrom_user tests Turn Anton's memcpy / copy_tofrom_user test into something that can live in tools/testing/selftests. It requires one turd in arch/powerpc/lib/memcpy_64.S, but it's pretty harmless IMHO. We are sailing very close to the wind with the feature macros. We define them to nothing, which currently means we get a few extra nops and include the unaligned calls. Signed-off-by: Anton Blanchard an...@samba.org Signed-off-by: Michael Ellerman m...@ellerman.id.au Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org cscope reports error when generating the cross-reference database: $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope GEN cscope cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S And when calling cscope from ./obj-cscope/ directory, it reports errors too. Hopefully it doesn't stop it from working, so I'm still able to use cscope to browse kernel sources. No, it won't stop it from working, it just won't search those files. I don't recall exactly the reason, but IIRC there was a big discussion long ago about symlinks and our ability to support them (around version 1.94 I think). We decided to not handle symlinks, as they would either point outside our search tree, which we didn't want to include, or would point to another file in the search tree, which made loading them pointless (as we would cover the search in the pointed file). Neil It's a rather uncommon side effect of having (for the first time ?) sources files as symlinks: looking for symlinks in the kernel sources returns only: $ find . -type l ./arch/mips/boot/dts/include/dt-bindings ./arch/microblaze/boot/dts/system.dts ./arch/powerpc/boot/dts/include/dt-bindings ./arch/metag/boot/dts/include/dt-bindings ./arch/arm/boot/dts/include/dt-bindings ./tools/testing/selftests/powerpc/copyloops/copyuser_power7.S ./tools/testing/selftests/powerpc/copyloops/memcpy_64.S ./tools/testing/selftests/powerpc/copyloops/memcpy_power7.S ./tools/testing/selftests/powerpc/copyloops/copyuser_64.S ./obj-cscope/source ./Documentation/DocBook/vidioc-g-sliced-vbi-cap.xml ./Documentation/DocBook/vidioc-decoder-cmd.xml ... ./Documentation/DocBook/media-func-ioctl.xml ./Documentation/DocBook/vidioc-enumoutput.xml So one can wonder if having symlinked sources files is an expected supported feature for kbuild and all the various kernel tools/infrastructure ? Regarding cscope specifically, it does not support symlink, and it's the expected behavior according to the bug reports I was able to find: #214 cscope ignores symlinks to files http://sourceforge.net/p/cscope/bugs/214/ #229 -I options doesn't handle symbolic link http://sourceforge.net/p/cscope/bugs/229/ #247 cscope: cannot find file http://sourceforge.net/p/cscope/bugs/247/ #252 cscope: cannot find file *** http://sourceforge.net/p/cscope/bugs/252/ #261 Regression - version 15.7a does not follow symbolic links http://sourceforge.net/p/cscope/bugs/261/ Regards. -- Yann Droneaud OPTEYA ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
On Mon, Apr 07, 2014 at 02:42:59PM +0200, Gerhard Sittig wrote: On Mon, 2014-04-07 at 06:42 -0400, Neil Horman wrote: On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote: [ ... ] cscope reports error when generating the cross-reference database: $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope GEN cscope cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S And when calling cscope from ./obj-cscope/ directory, it reports errors too. Hopefully it doesn't stop it from working, so I'm still able to use cscope to browse kernel sources. No, it won't stop it from working, it just won't search those files. I don't recall exactly the reason, but IIRC there was a big discussion long ago about symlinks and our ability to support them (around version 1.94 I think). We decided to not handle symlinks, as they would either point outside our search tree, which we didn't want to include, or would point to another file in the search tree, which made loading them pointless (as we would cover the search in the pointed file). So there are valid reasons to not process those filesystem entries. Would it be useful to not emit the warnings then? Or to silent those warnings when the user knows it's perfectly legal to skip those filesytem entries? Like what you can do with the ctags(1) command and its --links option. I would see no problem with an option to do that. I'd like to make it opt-in, so that people who want to know about symlink issues will still see them, but I'd be supportive of an option to quiet them Neil virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On Fri, Jul 26, 2013 at 11:19:13AM +1000, Anton Blanchard wrote: Hi Neil, Sorry I'm a bit late to the thread, I've ben swamped. Has someone tested this with kexec/kdump? Thats why the origional patch was created, because when kexec loads the kernel at a different physical address, the relocations messed with the module crc's, and modules couldn't load during the kexec boot. Assuming that kernaddr_start gets set appropriately during boot, using PHYSICAL_START should be fine, but I wanted to check, and don't currently have access to a powerpc system to do so. Neil I tested a relocatable kernel forced to run at a non zero physical address (ie basically kdump). I verified CRCs were bad with your original patch backed out, and were good with this patch applied. Anton Perfect, sounds like a sufficient test to me. Acked-by: Neil Horman nhor...@tuxdriver.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On Thu, Jul 25, 2013 at 09:14:25AM +1000, Benjamin Herrenschmidt wrote: On Thu, 2013-07-25 at 08:34 +1000, Anton Blanchard wrote: Apart from the annoying colors, is there anything specific I should be looking for? Some sort of error message, or output that actually makes sense? Thanks for testing! Ben, I think the patch is good to go. Sent it yesterday to Linus, it's upstream already :-) Cheers, Ben. Sorry I'm a bit late to the thread, I've ben swamped. Has someone tested this with kexec/kdump? Thats why the origional patch was created, because when kexec loads the kernel at a different physical address, the relocations messed with the module crc's, and modules couldn't load during the kexec boot. Assuming that kernaddr_start gets set appropriately during boot, using PHYSICAL_START should be fine, but I wanted to check, and don't currently have access to a powerpc system to do so. Neil ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] irqbalance, powerpc: add IRQs without settable SMP affinity to banned list
On Fri, Sep 24, 2010 at 04:56:34PM +1000, Michael Neuling wrote:
size_t size =3D 0;
FILE *file;
sprintf(buf, /proc/irq/%i/smp_affinity,
number);
- file =3D fopen(buf, r);
+ file =3D fopen(buf, r+);
if (!file)
continue;
if (getline(line, size, file)=3D=3D0) {
@@ -89,7 +89,14 @@
continue;
}
cpumask_parse_user(line, strlen(line),
irq-mask);
- fclose(file);
+ /*
+* Check that we can write the affinity, if
+* not take it out of the list.
+*/
+ if (fputs(line, file) =3D=3D EOF)
+ can_set =3D 0;
This is maybe a nit, but writing to the affinity file can fail for a few
different reasons, some of them permanent, some transient. For instance,=
if
we're in a memory constrained condition temporarily irq_affinity_proc_wri=
te
might return -ENOMEM. =20
Yeah true, usually followed shortly by your kernel going so far into
swap you never get it back, or OOMing, but I guess it's possible.
Might it be better to modify this code so that, instead
of using fputs to merge the various errors into an EOF, we use some other=
write
method that lets us better determine the error and selectively ban the in=
terrupt
only for those errors which we consider permanent?
Yep. It seems fputs() gives you know way to get the actual error from
write(), so it looks we'll need to switch to open/write, but that's
probably not so terrible.
fclose inherits the error from fputs and it sets errno correctly. Below
uses this to catch only EIO errors and mark them for the banned list.
Mikey
irqbalance, powerpc: add IRQs without settable SMP affinity to banned list
On pseries powerpc, IPIs are registered with an IRQ number so
/proc/interrupts looks like this on a 2 core/2 thread machine:
CPU0 CPU1 CPU2 CPU3
16:316428232905141138794 983121 XICS Level
IPI
18:2605674 0 304994 0 XICS Level
lan0
30: 400057 0 169209 0 XICS Level
ibmvscsi
LOC: 133734 77250 106425 91951 Local timer interrupts
SPU: 0 0 0 0 Spurious interrupts
CNT: 0 0 0 0 Performance monitoring
interrupts
MCE: 0 0 0 0 Machine check exceptions
Unfortunately this means irqbalance attempts to set the affinity of IPIs
which is not possible. So in the above case, when irqbalance is in
performance mode due to heavy IPI, lan0 and ibmvscsi activity, it
sometimes attempts to put the IPIs on one core (CPU01) and lan0 and
ibmvscsi on the other core (CPU23). This is suboptimal as we want lan0
and ibmvscsi to be on separate cores and IPIs to be ignored.
When irqblance attempts writes to the IPI smp_affinity (ie.
/proc/irq/16/smp_affinity in the above example) it fails with an EIO but
irqbalance currently ignores this.
This patch catches these write fails and in this case adds that IRQ
number to the banned IRQ list. This will catch the above IPI case and
any other IRQ where the SMP affinity can't be set.
Tested on POWER6, POWER7 and x86.
Signed-off-by: Michael Neuling mi...@neuling.org
Index: irqbalance/irqlist.c
===
--- irqbalance.orig/irqlist.c
+++ irqbalance/irqlist.c
@@ -28,6 +28,7 @@
#include unistd.h
#include sys/types.h
#include dirent.h
+#include errno.h
#include types.h
#include irqbalance.h
@@ -67,7 +68,7 @@
DIR *dir;
struct dirent *entry;
char *c, *c2;
- int nr , count = 0;
+ int nr , count = 0, can_set = 1;
char buf[PATH_MAX];
sprintf(buf, /proc/irq/%i, number);
dir = opendir(buf);
@@ -80,7 +81,7 @@
size_t size = 0;
FILE *file;
sprintf(buf, /proc/irq/%i/smp_affinity, number);
- file = fopen(buf, r);
+ file = fopen(buf, r+);
if (!file)
continue;
if (getline(line, size, file)==0) {
@@ -89,7 +90,13 @@
continue;
}
cpumask_parse_user(line, strlen(line), irq-mask);
- fclose(file);
+ /*
+ * Check that we can write the
Re: [PATCH] irqbalance, powerpc: add IRQs without settable SMP affinity to banned list
On Thu, Sep 23, 2010 at 08:57:20PM +1000, Michael Neuling wrote:
+ if (fwrite(line, strlen(line) - 1, 1, file) == 0)
if (fputs(line, file) == EOF)
Good point thanks... new patch below
Mikey
irqbalance, powerpc: add IRQs without settable SMP affinity to banned list
On pseries powerpc, IPIs are registered with an IRQ number so
/proc/interrupts looks like this on a 2 core/2 thread machine:
CPU0 CPU1 CPU2 CPU3
16:316428232905141138794 983121 XICS Level
IPI
18:2605674 0 304994 0 XICS Level
lan0
30: 400057 0 169209 0 XICS Level
ibmvscsi
LOC: 133734 77250 106425 91951 Local timer interrupts
SPU: 0 0 0 0 Spurious interrupts
CNT: 0 0 0 0 Performance monitoring
interrupts
MCE: 0 0 0 0 Machine check exceptions
Unfortunately this means irqbalance attempts to set the affinity of IPIs
which is not possible. So in the above case, when irqbalance is in
performance mode due to heavy IPI, lan0 and ibmvscsi activity, it
sometimes attempts to put the IPIs on one core (CPU01) and lan0 and
ibmvscsi on the other core (CPU23). This is suboptimal as we want lan0
and ibmvscsi to be on separate cores and IPIs to be ignored.
When irqblance attempts writes to the IPI smp_affinity (ie.
/proc/irq/16/smp_affinity in the above example) it fails but irqbalance
ignores currently ignores this.
This patch catches these write fails and in this case adds that IRQ
number to the banned IRQ list. This will catch the above IPI case and
any other IRQ where the SMP affinity can't be set.
Tested on POWER6, POWER7 and x86.
Signed-off-by: Michael Neuling mi...@neuling.org
Index: irqbalance/irqlist.c
===
--- irqbalance.orig/irqlist.c
+++ irqbalance/irqlist.c
@@ -67,7 +67,7 @@
DIR *dir;
struct dirent *entry;
char *c, *c2;
- int nr , count = 0;
+ int nr , count = 0, can_set = 1;
char buf[PATH_MAX];
sprintf(buf, /proc/irq/%i, number);
dir = opendir(buf);
@@ -80,7 +80,7 @@
size_t size = 0;
FILE *file;
sprintf(buf, /proc/irq/%i/smp_affinity, number);
- file = fopen(buf, r);
+ file = fopen(buf, r+);
if (!file)
continue;
if (getline(line, size, file)==0) {
@@ -89,7 +89,14 @@
continue;
}
cpumask_parse_user(line, strlen(line), irq-mask);
- fclose(file);
+ /*
+ * Check that we can write the affinity, if
+ * not take it out of the list.
+ */
+ if (fputs(line, file) == EOF)
+ can_set = 0;
This is maybe a nit, but writing to the affinity file can fail for a few
different reasons, some of them permanent, some transient. For instance, if
we're in a memory constrained condition temporarily irq_affinity_proc_write
might return -ENOMEM. Might it be better to modify this code so that, instead
of using fputs to merge the various errors into an EOF, we use some other write
method that lets us better determine the error and selectively ban the interrupt
only for those errors which we consider permanent?
Otherwise this looks fine to me.
Thanks
Neil
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Add vmcoreinfo symbols to allow makdumpfile to filter core files properly
On Thu, Aug 05, 2010 at 12:04:26PM +1000, Benjamin Herrenschmidt wrote: On Wed, 2010-08-04 at 10:49 -0400, Neil Horman wrote: Ping yet again. Ben, This needs review/acceptance from you or Paul Neil Isn't it already in powerpc-next about to be pulled by Linus ? Yes, there it is. Apologies. For whatever reason, I was looking on the main branch of your tree. It didn't occur to me to check your next branch. Sorry. In general, I recommend you check the status of your patches on patchwork. I'm nagging Jeremy to add a feature so it emails the submitter when the patch status changes :-) Noted, I'll remember that. Email from patchwork would be a nice feature. +1 from me. Thanks Regards Neil Cheers, Ben. ___ kexec mailing list ke...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Add vmcoreinfo symbols to allow makdumpfile to filter core files properly
On Tue, Jul 13, 2010 at 09:46:09AM -0400, Neil Horman wrote:
Hey all-
About 2 years ago now, I sent this patch upstream to allow makedumpfile
to properly filter cores on ppc64:
http://www.mail-archive.com/ke...@lists.infradead.org/msg02426.html
It got acks from the kexec folks so I pulled it into RHEL, but I never checked
back here to make sure it ever made it in, which apparently it didn't. It
still
needs to be included, so I'm reposting it here, making sure to copy all the
ppc
folks this time. I've retested it on the latest linus kernel and it works
fine,
allowing makedumpfile to find all the symbols it needs to properly strip a
vmcore on ppc64.
Neil
Signed-off-by: Neil Horman nhor...@tuxdriver.com
machine_kexec.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/kernel/machine_kexec.c
b/arch/powerpc/kernel/machine_kexec.c
index bb3d893..0df7031 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -45,6 +45,18 @@ void machine_kexec_cleanup(struct kimage *image)
ppc_md.machine_kexec_cleanup(image);
}
+void arch_crash_save_vmcoreinfo(void)
+{
+
+#ifdef CONFIG_NEED_MULTIPLE_NODES
+ VMCOREINFO_SYMBOL(node_data);
+ VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
+#endif
+#ifndef CONFIG_NEED_MULTIPLE_NODES
+ VMCOREINFO_SYMBOL(contig_page_data);
+#endif
+}
+
/*
* Do not allocate memory (or fail in any way) in machine_kexec().
* We are past the point of no return, committed to rebooting now.
___
kexec mailing list
ke...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Ping yet again. Ben, This needs review/acceptance from you or Paul
Neil
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Add vmcoreinfo symbols to allow makdumpfile to filter core files properly
On Tue, Jul 13, 2010 at 09:46:09AM -0400, Neil Horman wrote:
Hey all-
About 2 years ago now, I sent this patch upstream to allow makedumpfile
to properly filter cores on ppc64:
http://www.mail-archive.com/ke...@lists.infradead.org/msg02426.html
It got acks from the kexec folks so I pulled it into RHEL, but I never checked
back here to make sure it ever made it in, which apparently it didn't. It
still
needs to be included, so I'm reposting it here, making sure to copy all the
ppc
folks this time. I've retested it on the latest linus kernel and it works
fine,
allowing makedumpfile to find all the symbols it needs to properly strip a
vmcore on ppc64.
Neil
Signed-off-by: Neil Horman nhor...@tuxdriver.com
Ping, anyone want to chime in on this, its needed for dump filtering to work
properly on ppc64
Neil
machine_kexec.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/kernel/machine_kexec.c
b/arch/powerpc/kernel/machine_kexec.c
index bb3d893..0df7031 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -45,6 +45,18 @@ void machine_kexec_cleanup(struct kimage *image)
ppc_md.machine_kexec_cleanup(image);
}
+void arch_crash_save_vmcoreinfo(void)
+{
+
+#ifdef CONFIG_NEED_MULTIPLE_NODES
+ VMCOREINFO_SYMBOL(node_data);
+ VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
+#endif
+#ifndef CONFIG_NEED_MULTIPLE_NODES
+ VMCOREINFO_SYMBOL(contig_page_data);
+#endif
+}
+
/*
* Do not allocate memory (or fail in any way) in machine_kexec().
* We are past the point of no return, committed to rebooting now.
___
kexec mailing list
ke...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: Add vmcoreinfo symbols to allow makdumpfile to filter core files properly
Hey all-
About 2 years ago now, I sent this patch upstream to allow makedumpfile
to properly filter cores on ppc64:
http://www.mail-archive.com/ke...@lists.infradead.org/msg02426.html
It got acks from the kexec folks so I pulled it into RHEL, but I never checked
back here to make sure it ever made it in, which apparently it didn't. It still
needs to be included, so I'm reposting it here, making sure to copy all the ppc
folks this time. I've retested it on the latest linus kernel and it works fine,
allowing makedumpfile to find all the symbols it needs to properly strip a
vmcore on ppc64.
Neil
Signed-off-by: Neil Horman nhor...@tuxdriver.com
machine_kexec.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/kernel/machine_kexec.c
b/arch/powerpc/kernel/machine_kexec.c
index bb3d893..0df7031 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -45,6 +45,18 @@ void machine_kexec_cleanup(struct kimage *image)
ppc_md.machine_kexec_cleanup(image);
}
+void arch_crash_save_vmcoreinfo(void)
+{
+
+#ifdef CONFIG_NEED_MULTIPLE_NODES
+ VMCOREINFO_SYMBOL(node_data);
+ VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
+#endif
+#ifndef CONFIG_NEED_MULTIPLE_NODES
+ VMCOREINFO_SYMBOL(contig_page_data);
+#endif
+}
+
/*
* Do not allocate memory (or fail in any way) in machine_kexec().
* We are past the point of no return, committed to rebooting now.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ppc64: re-enable kexec to allow module loads with CONFIG_MODVERSIONS and CONFIG_RELOCATABLE turned on
Paul, Ben, given that Rusty hasn't come back with any opinion on this patch, do you feel comfortable merging it via the ppc tree? Currently the earlyinit routine is only compiled in and used for your arch, so I think its fairly benign. Sorry, I *did* track down the archives for linuxppc-dev, then find your post, then read your patch. But I didn't actually reply. Other than minor issues, there's one significant one: you shouldn't be trying to change rodata. It might work on PPC today, but it's poor form at least. How's this? Untested on ppc. I'll try grab a ppc64 system and test this soon. Other changes: 1) I also changed reloc_start to an array; this is a good idea for any linker-defined symbols so the compiler can't make assumptions about size. Ok, I'll certainly trust your linker skill over mine :) 2) local.h? How about module.h? Seems good to me 3) I don't think the extra . = 0 is necessary. Its not, I was just trying to be clear about where reloc_start was to be placed. 4) ARCH_USES_RELOC_ENTRIES isn't clear enough for me; I prefer ARCH_RELOCATE_KCRCTAB. Sounds good to me. I'll test this as soon as I'm able Thanks! Neil Just finished testing your patch Rusty, it all works quite well, Thanks! Will this be going in via your tree, or the ppc tree? Acked-by: Neil Horman nhor...@tuxdriver.com Neil ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ppc64: re-enable kexec to allow module loads with CONFIG_MODVERSIONS and CONFIG_RELOCATABLE turned on
Paul, Ben, given that Rusty hasn't come back with any opinion on this patch, do you feel comfortable merging it via the ppc tree? Currently the earlyinit routine is only compiled in and used for your arch, so I think its fairly benign. Sorry, I *did* track down the archives for linuxppc-dev, then find your post, then read your patch. But I didn't actually reply. Other than minor issues, there's one significant one: you shouldn't be trying to change rodata. It might work on PPC today, but it's poor form at least. How's this? Untested on ppc. I'll try grab a ppc64 system and test this soon. Other changes: 1) I also changed reloc_start to an array; this is a good idea for any linker-defined symbols so the compiler can't make assumptions about size. Ok, I'll certainly trust your linker skill over mine :) 2) local.h? How about module.h? Seems good to me 3) I don't think the extra . = 0 is necessary. Its not, I was just trying to be clear about where reloc_start was to be placed. 4) ARCH_USES_RELOC_ENTRIES isn't clear enough for me; I prefer ARCH_RELOCATE_KCRCTAB. Sounds good to me. I'll test this as soon as I'm able Thanks! Neil ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ppc64: re-enable kexec to allow module loads with CONFIG_MODVERSIONS and CONFIG_RELOCATABLE turned on
On Thu, Nov 19, 2009 at 02:52:16PM -0500, Neil Horman wrote: Hey there- Before anyone flames me for what a oddball solution this is, let me just say I'm trying to get the ball rolling here. I think there may be better solutions that can be impemented in reloc_64.S, but I've yet to make any of the ones I've tried work successfully. I'm open to suggestions, but this solution is the only one so far that I've been able to get to work. thanks :) Adjust crcs in __kcrctab_* sections if relocs are used with CONFIG_RELOCATABLE When CONFIG_MODVERSIONS and CONFIG_RELOCATABLE are enabled on powerpc platforms, kdump has been failing in a rather odd way. specifically modules will not install. This is because when validating the crcs of symbols that the module needs, the crc of the module never matches the crc that is stored in the kernel. The underlying cause of this is how CONFIG_MODVERSIONS is implemented, and how CONFIG_RELOCATABLE are implemented. with CONFIG_MODVERSIONS enabled, for every exported symbol in the kernel we emit 2 symbols, __crc_#sym which is declared extern and __kcrctab_##sym, which is placed in the __kcrctab section of the binary. The latter has its value set equal to the address of the former (recalling it is declared extern). After the object file is built, genksyms is run on the processed source, and crcs are computed for each exported symbol. genksyms then emits a linker script which defines each of the needed __crc_##sym symbols, and sets their addresses euqal to their respective crcs. This script is then used in a secondary link to the previously build object file, so that the crcs of the missing symbol can be validated on module insert. The problem here is that because __kcrctab_sym is set equal to __crc_##sym, a relocation entry is emitted by the compiler for the __kcrctab__##sym. Normally this is not a problem, since relocation on other arches is done without the aid of .rel.dyn sections. PPC however uses these relocations when CONFIG_RELOCATABLE is enabled. nominally, since addressing starts at 0 for ppc, its irrelevant, but if we start at a non-zero address (like we do when booting via kexec from reserved crashkernel memory), the ppc boot code iterates over the relocation entries, and winds up adding that relocation offset to all symbols, including the symbols that are actually the aforementioned crc values in the __kcrctab_* sections. This effectively corrupts the crcs and prevents any module loads from happening during a kdump. My solution is to 'undo' these relocations prior to boot up. If ARCH_USES_RELOC_ENTRIES is defined, we add a symbol at address zero to the linker script for that arch (I call it reloc_start, so that reloc_start = 0). This symbol will then indicate the relocation offset for any given boot. We also add an initcall to the module code that, during boot, scans the __kcrctab_* sections and subtracts reloc_start from every entry in those sections, restoring the appropriate crc value. I've verified that this allows kexec to work properly on ppc64 systems myself. Signed-off-by: Neil Horman nhor...@tuxdriver.com Paul, Ben, given that Rusty hasn't come back with any opinion on this patch, do you feel comfortable merging it via the ppc tree? Currently the earlyinit routine is only compiled in and used for your arch, so I think its fairly benign. Regards Neil ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ppc64: re-enable kexec to allow module loads with CONFIG_MODVERSIONS and CONFIG_RELOCATABLE turned on
On Thu, Nov 19, 2009 at 02:52:16PM -0500, Neil Horman wrote: Hey there- Before anyone flames me for what a oddball solution this is, let me just say I'm trying to get the ball rolling here. I think there may be better solutions that can be impemented in reloc_64.S, but I've yet to make any of the ones I've tried work successfully. I'm open to suggestions, but this solution is the only one so far that I've been able to get to work. thanks :) Adjust crcs in __kcrctab_* sections if relocs are used with CONFIG_RELOCATABLE When CONFIG_MODVERSIONS and CONFIG_RELOCATABLE are enabled on powerpc platforms, kdump has been failing in a rather odd way. specifically modules will not install. This is because when validating the crcs of symbols that the module needs, the crc of the module never matches the crc that is stored in the kernel. The underlying cause of this is how CONFIG_MODVERSIONS is implemented, and how CONFIG_RELOCATABLE are implemented. with CONFIG_MODVERSIONS enabled, for every exported symbol in the kernel we emit 2 symbols, __crc_#sym which is declared extern and __kcrctab_##sym, which is placed in the __kcrctab section of the binary. The latter has its value set equal to the address of the former (recalling it is declared extern). After the object file is built, genksyms is run on the processed source, and crcs are computed for each exported symbol. genksyms then emits a linker script which defines each of the needed __crc_##sym symbols, and sets their addresses euqal to their respective crcs. This script is then used in a secondary link to the previously build object file, so that the crcs of the missing symbol can be validated on module insert. The problem here is that because __kcrctab_sym is set equal to __crc_##sym, a relocation entry is emitted by the compiler for the __kcrctab__##sym. Normally this is not a problem, since relocation on other arches is done without the aid of .rel.dyn sections. PPC however uses these relocations when CONFIG_RELOCATABLE is enabled. nominally, since addressing starts at 0 for ppc, its irrelevant, but if we start at a non-zero address (like we do when booting via kexec from reserved crashkernel memory), the ppc boot code iterates over the relocation entries, and winds up adding that relocation offset to all symbols, including the symbols that are actually the aforementioned crc values in the __kcrctab_* sections. This effectively corrupts the crcs and prevents any module loads from happening during a kdump. My solution is to 'undo' these relocations prior to boot up. If ARCH_USES_RELOC_ENTRIES is defined, we add a symbol at address zero to the linker script for that arch (I call it reloc_start, so that reloc_start = 0). This symbol will then indicate the relocation offset for any given boot. We also add an initcall to the module code that, during boot, scans the __kcrctab_* sections and subtracts reloc_start from every entry in those sections, restoring the appropriate crc value. I've verified that this allows kexec to work properly on ppc64 systems myself. Signed-off-by: Neil Horman nhor...@tuxdriver.com Ping, any thoughts here? As I've been mulling this over, I'm beginning to like this solution better than a completely arch-specific section, as this approach makes common the bits that any arch is going to need if they implement CONFIG_RELOCATABLE with a .rel[a].* section set. The alternative is of course to simply skip the appropriate relocations, but thats something that every arch will need to discover on their own. This makes it a bit more clear whats happening I think. Regards Neil ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ppc64: re-enable kexec to allow module loads with CONFIG_MODVERSIONS and CONFIG_RELOCATABLE turned on
Neil Horman writes: Before anyone flames me for what a oddball solution this is, let me just say I'm trying to get the ball rolling here. I think there may be better solutions that can be impemented in reloc_64.S, but I've yet to make any of the ones I've tried work successfully. I'm open to suggestions, but this solution is the only one so far that I've been able to get to work. thanks :) I had thought that the CRCs would be the only things with reloc addends less than 4G, but it turns out that the toolchain folds expressions like __pa(sym) into just a load from a doubleword (in the TOC) containing the physical address of sym. That needs to be relocated and its reloc addend will be less than 4GB. Hence the crashes early in boot that you were seeing with my proposed patch to reloc_64.S. Given that, your solution seems as reasonable as any. Should this go via the modules maintainer, Rusty Russell, perhaps? In any case, Acked-by: Paul Mackerras paulus at samba.org I'm not 100% sure, it does kind of split the middle in regards to what tree is should come through. Rusy, given the previous entries in this thread, do you have any thoughts on the patch, or which tree it should go through? I kind of think that the ppc tree is just fine here since its currently the only user of this code, but I'll defer to other opinions on the subject. Thanks Regards Neil ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] ppc64: re-enable kexec to allow module loads with CONFIG_MODVERSIONS and CONFIG_RELOCATABLE turned on
Hey there-
Before anyone flames me for what a oddball solution this is, let me just
say I'm trying to get the ball rolling here. I think there may be better
solutions that can be impemented in reloc_64.S, but I've yet to make any of the
ones I've tried work successfully. I'm open to suggestions, but this solution
is the only one so far that I've been able to get to work. thanks :)
Adjust crcs in __kcrctab_* sections if relocs are used with CONFIG_RELOCATABLE
When CONFIG_MODVERSIONS and CONFIG_RELOCATABLE are enabled on powerpc platforms,
kdump has been failing in a rather odd way. specifically modules will not
install. This is because when validating the crcs of symbols that the module
needs, the crc of the module never matches the crc that is stored in the kernel.
The underlying cause of this is how CONFIG_MODVERSIONS is implemented, and how
CONFIG_RELOCATABLE are implemented. with CONFIG_MODVERSIONS enabled, for every
exported symbol in the kernel we emit 2 symbols, __crc_#sym which is declared
extern and __kcrctab_##sym, which is placed in the __kcrctab section of the
binary. The latter has its value set equal to the address of the former
(recalling it is declared extern). After the object file is built, genksyms is
run on the processed source, and crcs are computed for each exported symbol.
genksyms then emits a linker script which defines each of the needed __crc_##sym
symbols, and sets their addresses euqal to their respective crcs. This script
is then used in a secondary link to the previously build object file, so that
the crcs of the missing symbol can be validated on module insert.
The problem here is that because __kcrctab_sym is set equal to __crc_##sym, a
relocation entry is emitted by the compiler for the __kcrctab__##sym. Normally
this is not a problem, since relocation on other arches is done without the aid
of .rel.dyn sections. PPC however uses these relocations when
CONFIG_RELOCATABLE is enabled. nominally, since addressing starts at 0 for ppc,
its irrelevant, but if we start at a non-zero address (like we do when booting
via kexec from reserved crashkernel memory), the ppc boot code iterates over the
relocation entries, and winds up adding that relocation offset to all symbols,
including the symbols that are actually the aforementioned crc values in the
__kcrctab_* sections. This effectively corrupts the crcs and prevents any
module loads from happening during a kdump.
My solution is to 'undo' these relocations prior to boot up. If
ARCH_USES_RELOC_ENTRIES is defined, we add a symbol at address zero to the
linker script for that arch (I call it reloc_start, so that reloc_start = 0).
This symbol will then indicate the relocation offset for any given boot. We
also add an initcall to the module code that, during boot, scans the __kcrctab_*
sections and subtracts reloc_start from every entry in those sections,
restoring the appropriate crc value.
I've verified that this allows kexec to work properly on ppc64 systems myself.
Signed-off-by: Neil Horman nhor...@tuxdriver.com
arch/powerpc/include/asm/local.h |6 ++
arch/powerpc/kernel/vmlinux.lds.S |4
kernel/module.c | 30 ++
3 files changed, 40 insertions(+)
diff --git a/arch/powerpc/include/asm/local.h b/arch/powerpc/include/asm/local.h
index 84b457a..9cc49e5 100644
--- a/arch/powerpc/include/asm/local.h
+++ b/arch/powerpc/include/asm/local.h
@@ -4,6 +4,12 @@
#include linux/percpu.h
#include asm/atomic.h
+#ifdef CONFIG_MODVERSIONS
+#define ARCH_USES_RELOC_ENTRIES
+
+extern unsigned long reloc_start;
+#endif
+
typedef struct
{
atomic_long_t a;
diff --git a/arch/powerpc/kernel/vmlinux.lds.S
b/arch/powerpc/kernel/vmlinux.lds.S
index 27735a7..2b9fb2e 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -38,6 +38,10 @@ jiffies = jiffies_64 + 4;
#endif
SECTIONS
{
+ . = 0;
+ reloc_start = .;
+ . = 0;
+
. = KERNELBASE;
/*
diff --git a/kernel/module.c b/kernel/module.c
index 8b7d880..87a4928 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -181,8 +181,11 @@ extern const struct kernel_symbol
__stop___ksymtab_gpl_future[];
extern const struct kernel_symbol __start___ksymtab_gpl_future[];
extern const struct kernel_symbol __stop___ksymtab_gpl_future[];
extern const unsigned long __start___kcrctab[];
+extern const unsigned long __stop___kcrctab[];
extern const unsigned long __start___kcrctab_gpl[];
+extern const unsigned long __stop___kcrctab_gpl[];
extern const unsigned long __start___kcrctab_gpl_future[];
+extern const unsigned long __stop___kcrctab_gpl_future[];
#ifdef CONFIG_UNUSED_SYMBOLS
extern const struct kernel_symbol __start___ksymtab_unused[];
extern const struct kernel_symbol __stop___ksymtab_unused[];
@@ -3144,3 +3147,30 @@ int module_get_iter_tracepoints(struct tracepoint_iter
*iter)
return found;
}
#endif
+
+#ifdef
Re: [PATCH] Do not inline putprops function
On Wed, Jun 17, 2009 at 10:26:35PM +1000, Michael Ellerman wrote: On Wed, 2009-06-17 at 17:29 +0530, M. Mohan Kumar wrote: Do not inline putprops function With the recent kexec-tools git tree, both kexec and kdump kernels hang (i.e kexec -l and kexec -p respectively). This happened after the patch ppc64: cleanups commit b43a84a31a4be6ed025c1bdef3bb1c3c12e01b16. I tried reverting each hunk and then found out that retaining following lines in fs2dt.c makes kexec/kdump work. -static unsigned *dt_len; /* changed len of modified cmdline - in flat device-tree */ [] - dt_len = dt; I don't have any clue why removing a unused variable would cause the kexec kernel to hang. After further investigation, I observed that if the putprops function is not inlined, kexec/kdump kernel would work even after removing the above lines. This patch directs gcc to not inline the putprops function. Now we could invoke kexec and kdump kernels. What compiler version are you using? Does the behaviour change if you use a newer/older compiler? It sounds to me like there's some deeper bug and your patch is just papering over it. Agreed, this doesn't make any sense. Try changing the compiler version to see if the problem goes away or stops. It might also be worthwhile to dump the contents of the device tree at the start and end of the kexec process. If the changing of how a function is inlined is causing a hang, its likely changing how the putprops function is writing information to the device tree. Understanding what that change is will likely provide clues to how the code has changed. Neil cheers ___ kexec mailing list ke...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Do not inline putprops function
On Wed, Jun 17, 2009 at 07:04:35PM +0530, M. Mohan Kumar wrote: On Wed, Jun 17, 2009 at 09:04:13AM -0400, Neil Horman wrote: On Wed, Jun 17, 2009 at 10:26:35PM +1000, Michael Ellerman wrote: What compiler version are you using? Does the behaviour change if you use a newer/older compiler? It sounds to me like there's some deeper bug and your patch is just papering over it. I tried with gcc 4.3.2. Let me try with a recent version and update. Agreed, this doesn't make any sense. Try changing the compiler version to see if the problem goes away or stops. It might also be worthwhile to dump the contents of the device tree at the start and end of the kexec process. If the changing of how a function is inlined is causing a hang, its likely changing how the putprops function is writing information to the device tree. Understanding what that change is will likely provide clues to how the code has changed. Neil, there was no code change in fs2dt.c Sure there was, by modifying the code to tell it to not inline the putprops function, you changed how the complier optimizes that code block. There may not be any source level code change, but the assembly is certainly different, and it must be so in a way thats causing a hang. The putpops function changes the contents of the ppc64 device-tree, so if this is a compiler bug, and its causing a hang, I imagine we'll see some manifestation in a change in the device tree contents. Neil Regards, M. Mohan Kumar ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Do not inline putprops function
On Wed, Jun 17, 2009 at 07:56:52PM +0530, M. Mohan Kumar wrote: On Wed, Jun 17, 2009 at 10:05:14AM -0400, Neil Horman wrote: On Wed, Jun 17, 2009 at 07:04:35PM +0530, M. Mohan Kumar wrote: On Wed, Jun 17, 2009 at 09:04:13AM -0400, Neil Horman wrote: On Wed, Jun 17, 2009 at 10:26:35PM +1000, Michael Ellerman wrote: What compiler version are you using? Does the behaviour change if you use a newer/older compiler? It sounds to me like there's some deeper bug and your patch is just papering over it. I tried with gcc 4.3.2. Let me try with a recent version and update. Agreed, this doesn't make any sense. Try changing the compiler version to see if the problem goes away or stops. It might also be worthwhile to dump the contents of the device tree at the start and end of the kexec process. If the changing of how a function is inlined is causing a hang, its likely changing how the putprops function is writing information to the device tree. Understanding what that change is will likely provide clues to how the code has changed. Neil, there was no code change in fs2dt.c Sure there was, by modifying the code to tell it to not inline the putprops function, you changed how the complier optimizes that code block. There may not be any source level code change, but the assembly is certainly different, and it must be so in a way thats causing a hang. The putpops function changes the contents of the ppc64 device-tree, so if this is a compiler bug, and its causing a hang, I imagine we'll see some manifestation in a change in the device tree contents. As I mentioned in the patch description, when I retained the variable dt_len and dt_len = dt assignment, kexec/kdump kernel would work. But even if I comment the dt_len = dt assignment kernel would hang. If you need I can Yes, and as Michael has noted, that is likey an indication of a compiler bug. You shouldn't get differing behavior in a kdump kernel by removing an unused variable in kexec. Which is why I'm unconvinced this patch is really fixing anything, but rather just avoiding the real problem. send objdump of fs2dt.o with and without this assignment. That would be a fine thing to do, and I'd be happy to compare them. My though regarding the comparison of the device tree on a good and bad run was meant to expidite what change in the assembly we'd be looking for. If its the kdump kernel boot thats hanging, Its likely hanging on something in the device tree, as thats whats being manipulated by this code. So I figure that understanding whats changed there will point us toward what change in the assembly might be responsible for the hang. The assmebly's going to be signficantly different (lots of optimization might be lost from no longer inlining a function), so anything that helps us narrow down whats changed will be good Neil Regards, M. Mohan Kumar. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
