| Message ID | 1420678479-630-1-git-send-email-tyler.baker@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tyler, Am Mittwoch, den 07.01.2015, 16:54 -0800 schrieb Tyler Baker: > Call spin_lock_init() before the spinlocks are used, preventing a lockdep > splat. > > I have been observing lockdep complaining [1] during boot on my a80 optimus [2] > when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, > and has been tested on a few other sunxi platforms without issue. > > [1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html > [2] http://kernelci.org/boot/?a80-optimus > > Signed-off-by: Tyler Baker <tyler.baker@linaro.org> > --- > This patch was tested on 3.19.0-rc3 Applied, thanks! regards Philipp
Hi Tyler, On Wed, Jan 07, 2015 at 04:54:39PM -0800, Tyler Baker wrote: > Call spin_lock_init() before the spinlocks are used, preventing a lockdep > splat. > > I have been observing lockdep complaining [1] during boot on my a80 optimus [2] > when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, > and has been tested on a few other sunxi platforms without issue. > > [1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html > [2] http://kernelci.org/boot/?a80-optimus > > Signed-off-by: Tyler Baker <tyler.baker@linaro.org> Thanks for this patch. Can you send this to stable too please? Maxime
Hi Maxime, On 8 January 2015 at 01:30, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi Tyler, > > On Wed, Jan 07, 2015 at 04:54:39PM -0800, Tyler Baker wrote: >> Call spin_lock_init() before the spinlocks are used, preventing a lockdep >> splat. >> >> I have been observing lockdep complaining [1] during boot on my a80 optimus [2] >> when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, >> and has been tested on a few other sunxi platforms without issue. >> >> [1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html >> [2] http://kernelci.org/boot/?a80-optimus >> >> Signed-off-by: Tyler Baker <tyler.baker@linaro.org> > > Thanks for this patch. You're welcome. > > Can you send this to stable too please? Sure. Once it has been merged in Linus' tree I'll send it for -stable review. > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com Cheers,
Hi Tyler, Maxime, On 01/08/2015 01:54 AM, Tyler Baker wrote: > Call spin_lock_init() before the spinlocks are used, preventing a lockdep > splat. > > I have been observing lockdep complaining [1] during boot on my a80 optimus [2] > when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, > and has been tested on a few other sunxi platforms without issue. > > [1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html > [2] http://kernelci.org/boot/?a80-optimus > > Signed-off-by: Tyler Baker <tyler.baker@linaro.org> > --- > This patch was tested on 3.19.0-rc3 > > drivers/reset/reset-sunxi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c > index eebc52c..e37250c 100644 > --- a/drivers/reset/reset-sunxi.c > +++ b/drivers/reset/reset-sunxi.c > @@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device *pdev) > if (IS_ERR(data->membase)) > return PTR_ERR(data->membase); > > + spin_lock_init(&data->lock); > + Shouldn't the lock be initialized also in early init function (sunxi_reset_init) ? Regards, Maxime > data->rcdev.owner = THIS_MODULE; > data->rcdev.nr_resets = resource_size(res) * 32; > data->rcdev.ops = &sunxi_reset_ops;
With Maxime in the To: list. On 01/09/2015 12:59 PM, Maxime Coquelin wrote: > Hi Tyler, Maxime, > > On 01/08/2015 01:54 AM, Tyler Baker wrote: >> Call spin_lock_init() before the spinlocks are used, preventing a >> lockdep >> splat. >> >> I have been observing lockdep complaining [1] during boot on my a80 >> optimus [2] >> when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the >> splat, >> and has been tested on a few other sunxi platforms without issue. >> >> [1] >> http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html >> [2] http://kernelci.org/boot/?a80-optimus >> >> Signed-off-by: Tyler Baker <tyler.baker@linaro.org> >> --- >> This patch was tested on 3.19.0-rc3 >> >> drivers/reset/reset-sunxi.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c >> index eebc52c..e37250c 100644 >> --- a/drivers/reset/reset-sunxi.c >> +++ b/drivers/reset/reset-sunxi.c >> @@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct >> platform_device *pdev) >> if (IS_ERR(data->membase)) >> return PTR_ERR(data->membase); >> + spin_lock_init(&data->lock); >> + > Shouldn't the lock be initialized also in early init function > (sunxi_reset_init) ? > > Regards, > Maxime >> data->rcdev.owner = THIS_MODULE; >> data->rcdev.nr_resets = resource_size(res) * 32; >> data->rcdev.ops = &sunxi_reset_ops; >
Hi Maxime, On 9 January 2015 at 06:44, Maxime Coquelin <maxime.coquelin@st.com> wrote: > With Maxime in the To: list. > > > > On 01/09/2015 12:59 PM, Maxime Coquelin wrote: >> >> Hi Tyler, Maxime, >> >> On 01/08/2015 01:54 AM, Tyler Baker wrote: >>> >>> Call spin_lock_init() before the spinlocks are used, preventing a lockdep >>> splat. >>> >>> I have been observing lockdep complaining [1] during boot on my a80 >>> optimus [2] >>> when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the >>> splat, >>> and has been tested on a few other sunxi platforms without issue. >>> >>> [1] >>> http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html >>> [2] http://kernelci.org/boot/?a80-optimus >>> >>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org> >>> --- >>> This patch was tested on 3.19.0-rc3 >>> >>> drivers/reset/reset-sunxi.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c >>> index eebc52c..e37250c 100644 >>> --- a/drivers/reset/reset-sunxi.c >>> +++ b/drivers/reset/reset-sunxi.c >>> @@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device >>> *pdev) >>> if (IS_ERR(data->membase)) >>> return PTR_ERR(data->membase); >>> + spin_lock_init(&data->lock); >>> + >> >> Shouldn't the lock be initialized also in early init function >> (sunxi_reset_init) ? Thanks for pointing this out. It seems reasonable to me to add this for the sun6i platforms that use the early init function. Currently trying to track down a sun6i platform so I can run a few locking validation tests. >> >> Regards, >> Maxime >>> >>> data->rcdev.owner = THIS_MODULE; >>> data->rcdev.nr_resets = resource_size(res) * 32; >>> data->rcdev.ops = &sunxi_reset_ops; >> >> >
On 9 January 2015 at 12:44, Tyler Baker <tyler.baker@linaro.org> wrote: > Hi Maxime, > > On 9 January 2015 at 06:44, Maxime Coquelin <maxime.coquelin@st.com> wrote: >> With Maxime in the To: list. >> >> >> >> On 01/09/2015 12:59 PM, Maxime Coquelin wrote: >>> >>> Hi Tyler, Maxime, >>> >>> On 01/08/2015 01:54 AM, Tyler Baker wrote: >>>> >>>> Call spin_lock_init() before the spinlocks are used, preventing a lockdep >>>> splat. >>>> >>>> I have been observing lockdep complaining [1] during boot on my a80 >>>> optimus [2] >>>> when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the >>>> splat, >>>> and has been tested on a few other sunxi platforms without issue. >>>> >>>> [1] >>>> http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html >>>> [2] http://kernelci.org/boot/?a80-optimus >>>> >>>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org> >>>> --- >>>> This patch was tested on 3.19.0-rc3 >>>> >>>> drivers/reset/reset-sunxi.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c >>>> index eebc52c..e37250c 100644 >>>> --- a/drivers/reset/reset-sunxi.c >>>> +++ b/drivers/reset/reset-sunxi.c >>>> @@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device >>>> *pdev) >>>> if (IS_ERR(data->membase)) >>>> return PTR_ERR(data->membase); >>>> + spin_lock_init(&data->lock); >>>> + >>> >>> Shouldn't the lock be initialized also in early init function >>> (sunxi_reset_init) ? > > Thanks for pointing this out. It seems reasonable to me to add this > for the sun6i platforms that use the early init function. Currently > trying to track down a sun6i platform so I can run a few locking > validation tests. Olof was kind enough to volunteer his sun6i platform (colombus) and provide the results. The results confirms there is a need to also initialize the lock in sunxi_reset_init as Maxime C. mentioned. DEBUG: sunxi_reset_init() DEBUG: sunxi_reset_deassert() INFO: trying to register non-static key. the code is fine but needs lockdep annotation. turning off the locking correctness validator. CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.19.0-rc3-00039-gccd0de2-dirty #5 Hardware name: Allwinner sun6i (A31) Family [<c021679c>] (unwind_backtrace) from [<c0212140>] (show_stack+0x10/0x14) [<c0212140>] (show_stack) from [<c09557c4>] (dump_stack+0x8c/0x9c) [<c09557c4>] (dump_stack) from [<c028293c>] (__lock_acquire+0x1ef4/0x1f00) [<c028293c>] (__lock_acquire) from [<c028311c>] (lock_acquire+0x6c/0x8c) [<c028311c>] (lock_acquire) from [<c095b810>] (_raw_spin_lock_irqsave+0x3c/0x50) [<c095b810>] (_raw_spin_lock_irqsave) from [<c0542d80>] (sunxi_reset_deassert+0x2c/0x88) [<c0542d80>] (sunxi_reset_deassert) from [<c0d39a00>] (sun5i_timer_init+0xc0/0x1f0) [<c0d39a00>] (sun5i_timer_init) from [<c0d3897c>] (clocksource_of_init+0x4c/0x8c) [<c0d3897c>] (clocksource_of_init) from [<c0cf0b34>] (start_kernel+0x278/0x3c8) [<c0cf0b34>] (start_kernel) from [<40208074>] (0x40208074) I'll go ahead and resend an updated patch and cc stable if Philipp and Maxime R. they agree. > >>> >>> Regards, >>> Maxime >>>> >>>> data->rcdev.owner = THIS_MODULE; >>>> data->rcdev.nr_resets = resource_size(res) * 32; >>>> data->rcdev.ops = &sunxi_reset_ops; >>> >>> >> > > > > -- > Tyler Baker > Tech Lead, LAVA > Linaro.org | Open source software for ARM SoCs > Follow Linaro: http://www.facebook.com/pages/Linaro > http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog Cheers,
Hi Tyler, Am Freitag, den 09.01.2015, 16:22 -0800 schrieb Tyler Baker: > I'll go ahead and resend an updated patch and cc stable if Philipp and > Maxime R. they agree. Please do. thanks Philipp
Hi Tyler, On 01/10/2015 01:22 AM, Tyler Baker wrote: > On 9 January 2015 at 12:44, Tyler Baker <tyler.baker@linaro.org> wrote: >> Hi Maxime, >> >> On 9 January 2015 at 06:44, Maxime Coquelin <maxime.coquelin@st.com> wrote: >>> With Maxime in the To: list. >>> >>> >>> >>> On 01/09/2015 12:59 PM, Maxime Coquelin wrote: >>>> Hi Tyler, Maxime, >>>> >>>> On 01/08/2015 01:54 AM, Tyler Baker wrote: >>>>> Call spin_lock_init() before the spinlocks are used, preventing a lockdep >>>>> splat. >>>>> >>>>> I have been observing lockdep complaining [1] during boot on my a80 >>>>> optimus [2] >>>>> when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the >>>>> splat, >>>>> and has been tested on a few other sunxi platforms without issue. >>>>> >>>>> [1] >>>>> http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html >>>>> [2] http://kernelci.org/boot/?a80-optimus >>>>> >>>>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org> >>>>> --- >>>>> This patch was tested on 3.19.0-rc3 >>>>> >>>>> drivers/reset/reset-sunxi.c | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c >>>>> index eebc52c..e37250c 100644 >>>>> --- a/drivers/reset/reset-sunxi.c >>>>> +++ b/drivers/reset/reset-sunxi.c >>>>> @@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device >>>>> *pdev) >>>>> if (IS_ERR(data->membase)) >>>>> return PTR_ERR(data->membase); >>>>> + spin_lock_init(&data->lock); >>>>> + >>>> Shouldn't the lock be initialized also in early init function >>>> (sunxi_reset_init) ? >> Thanks for pointing this out. It seems reasonable to me to add this >> for the sun6i platforms that use the early init function. Currently >> trying to track down a sun6i platform so I can run a few locking >> validation tests. > Olof was kind enough to volunteer his sun6i platform (colombus) and > provide the results. The results confirms there is a need to also > initialize the lock in sunxi_reset_init as Maxime C. mentioned. > > DEBUG: sunxi_reset_init() > DEBUG: sunxi_reset_deassert() > INFO: trying to register non-static key. > the code is fine but needs lockdep annotation. > turning off the locking correctness validator. > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.19.0-rc3-00039-gccd0de2-dirty #5 > Hardware name: Allwinner sun6i (A31) Family > [<c021679c>] (unwind_backtrace) from [<c0212140>] (show_stack+0x10/0x14) > [<c0212140>] (show_stack) from [<c09557c4>] (dump_stack+0x8c/0x9c) > [<c09557c4>] (dump_stack) from [<c028293c>] (__lock_acquire+0x1ef4/0x1f00) > [<c028293c>] (__lock_acquire) from [<c028311c>] (lock_acquire+0x6c/0x8c) > [<c028311c>] (lock_acquire) from [<c095b810>] (_raw_spin_lock_irqsave+0x3c/0x50) > [<c095b810>] (_raw_spin_lock_irqsave) from [<c0542d80>] > (sunxi_reset_deassert+0x2c/0x88) > [<c0542d80>] (sunxi_reset_deassert) from [<c0d39a00>] > (sun5i_timer_init+0xc0/0x1f0) > [<c0d39a00>] (sun5i_timer_init) from [<c0d3897c>] > (clocksource_of_init+0x4c/0x8c) > [<c0d3897c>] (clocksource_of_init) from [<c0cf0b34>] (start_kernel+0x278/0x3c8) > [<c0cf0b34>] (start_kernel) from [<40208074>] (0x40208074) > > I'll go ahead and resend an updated patch and cc stable if Philipp and > Maxime R. they agree. Thanks for having done the test. I have no Allwinner HW to try. Best regards, Maxime > >>>> Regards, >>>> Maxime >>>>> data->rcdev.owner = THIS_MODULE; >>>>> data->rcdev.nr_resets = resource_size(res) * 32; >>>>> data->rcdev.ops = &sunxi_reset_ops; >>>> >> >> >> -- >> Tyler Baker >> Tech Lead, LAVA >> Linaro.org | Open source software for ARM SoCs >> Follow Linaro: http://www.facebook.com/pages/Linaro >> http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog > Cheers, >
diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c index eebc52c..e37250c 100644 --- a/drivers/reset/reset-sunxi.c +++ b/drivers/reset/reset-sunxi.c @@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device *pdev) if (IS_ERR(data->membase)) return PTR_ERR(data->membase); + spin_lock_init(&data->lock); + data->rcdev.owner = THIS_MODULE; data->rcdev.nr_resets = resource_size(res) * 32; data->rcdev.ops = &sunxi_reset_ops;
Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat. I have been observing lockdep complaining [1] during boot on my a80 optimus [2] when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, and has been tested on a few other sunxi platforms without issue. [1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html [2] http://kernelci.org/boot/?a80-optimus Signed-off-by: Tyler Baker <tyler.baker@linaro.org> --- This patch was tested on 3.19.0-rc3 drivers/reset/reset-sunxi.c | 2 ++ 1 file changed, 2 insertions(+)