All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Cc: linux-rt-users-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Paul Gortmaker
	<paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>,
	"linux-next-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-next-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	sparclinux-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sam Ravnborg <sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org>
Subject: Re: [PATCH next v2] OF: convert devtree lock from rw_lock to raw spinlock
Date: Mon, 11 Feb 2013 12:29:09 -0700	[thread overview]
Message-ID: <51194685.5010904@wwwdotorg.org> (raw)
In-Reply-To: <511585A9.9090008-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 02/08/2013 04:09 PM, Rob Herring wrote:
> On 02/06/2013 02:30 PM, Paul Gortmaker wrote:
>> From: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>>
>> With the locking cleanup in place (from "OF: Fixup resursive
>> locking code paths"), we can now do the conversion from the
>> rw_lock to a raw spinlock as required for preempt-rt.
>>
>> The previous cleanup and this conversion were originally
>> separate since they predated when mainline got raw spinlock (in
>> commit c2f21ce2e31286a "locking: Implement new raw_spinlock").
>>
>> So, at that point in time, the cleanup was considered plausible
>> for mainline, but not this conversion.  In any case, we've kept
>> them separate as it makes for easier review and better bisection.
>>
>> Signed-off-by: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>> [PG: taken from preempt-rt, update subject & add a commit log]
>> Signed-off-by: Paul Gortmaker <paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
>> ---
>>
>> [v2: recent commit e81b329 ("powerpc+of: Add /proc device tree
>>  updating to of node add/remove") added two more instances of
>>  write_unlock that also needed converting to raw_spin_unlock.
>>  Retested (boot) on sbc8548, defconfig builds on arm/sparc; no
>>  new warnings observed.]
>>
>>  arch/sparc/kernel/prom_common.c |   4 +-
>>  drivers/of/base.c               | 100 ++++++++++++++++++++++------------------
>>  include/linux/of.h              |   2 +-
>>  3 files changed, 59 insertions(+), 47 deletions(-)
> 
> Applied.

This commit is present in next-20130211, and causes a boot failure
(hang) early while booting on Tegra. Reverting just this one commit
solves the issue.

I'll see if I can track down where the issue is. Given the commit
description, I assume there's some new recursive lock issue that snuck
in between the previous fix for them and this commit? Any hints welcome.

One thing I wonder looking at the patch: Most paths use
raw_spin_lock_irqsave() but a few use just raw_spin_lock(). I wonder how
that decision was made?

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Cc: linux-rt-users-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Paul Gortmaker
	<paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>,
	"linux-next-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-next-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	sparclinux-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sam Ravnborg <sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org>
Subject: Re: [PATCH next v2] OF: convert devtree lock from rw_lock to raw spinlock
Date: Mon, 11 Feb 2013 19:29:09 +0000	[thread overview]
Message-ID: <51194685.5010904@wwwdotorg.org> (raw)
In-Reply-To: <511585A9.9090008-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 02/08/2013 04:09 PM, Rob Herring wrote:
> On 02/06/2013 02:30 PM, Paul Gortmaker wrote:
>> From: Thomas Gleixner <tglx@linutronix.de>
>>
>> With the locking cleanup in place (from "OF: Fixup resursive
>> locking code paths"), we can now do the conversion from the
>> rw_lock to a raw spinlock as required for preempt-rt.
>>
>> The previous cleanup and this conversion were originally
>> separate since they predated when mainline got raw spinlock (in
>> commit c2f21ce2e31286a "locking: Implement new raw_spinlock").
>>
>> So, at that point in time, the cleanup was considered plausible
>> for mainline, but not this conversion.  In any case, we've kept
>> them separate as it makes for easier review and better bisection.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> [PG: taken from preempt-rt, update subject & add a commit log]
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> ---
>>
>> [v2: recent commit e81b329 ("powerpc+of: Add /proc device tree
>>  updating to of node add/remove") added two more instances of
>>  write_unlock that also needed converting to raw_spin_unlock.
>>  Retested (boot) on sbc8548, defconfig builds on arm/sparc; no
>>  new warnings observed.]
>>
>>  arch/sparc/kernel/prom_common.c |   4 +-
>>  drivers/of/base.c               | 100 ++++++++++++++++++++++------------------
>>  include/linux/of.h              |   2 +-
>>  3 files changed, 59 insertions(+), 47 deletions(-)
> 
> Applied.

This commit is present in next-20130211, and causes a boot failure
(hang) early while booting on Tegra. Reverting just this one commit
solves the issue.

I'll see if I can track down where the issue is. Given the commit
description, I assume there's some new recursive lock issue that snuck
in between the previous fix for them and this commit? Any hints welcome.

One thing I wonder looking at the patch: Most paths use
raw_spin_lock_irqsave() but a few use just raw_spin_lock(). I wonder how
that decision was made?

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Rob Herring <robherring2@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
	linux-rt-users@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,
	Sam Ravnborg <sam@ravnborg.org>,
	"linux-next@vger.kernel.org" <linux-next@vger.kernel.org>
Subject: Re: [PATCH next v2] OF: convert devtree lock from rw_lock to raw spinlock
Date: Mon, 11 Feb 2013 12:29:09 -0700	[thread overview]
Message-ID: <51194685.5010904@wwwdotorg.org> (raw)
In-Reply-To: <511585A9.9090008@gmail.com>

On 02/08/2013 04:09 PM, Rob Herring wrote:
> On 02/06/2013 02:30 PM, Paul Gortmaker wrote:
>> From: Thomas Gleixner <tglx@linutronix.de>
>>
>> With the locking cleanup in place (from "OF: Fixup resursive
>> locking code paths"), we can now do the conversion from the
>> rw_lock to a raw spinlock as required for preempt-rt.
>>
>> The previous cleanup and this conversion were originally
>> separate since they predated when mainline got raw spinlock (in
>> commit c2f21ce2e31286a "locking: Implement new raw_spinlock").
>>
>> So, at that point in time, the cleanup was considered plausible
>> for mainline, but not this conversion.  In any case, we've kept
>> them separate as it makes for easier review and better bisection.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> [PG: taken from preempt-rt, update subject & add a commit log]
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> ---
>>
>> [v2: recent commit e81b329 ("powerpc+of: Add /proc device tree
>>  updating to of node add/remove") added two more instances of
>>  write_unlock that also needed converting to raw_spin_unlock.
>>  Retested (boot) on sbc8548, defconfig builds on arm/sparc; no
>>  new warnings observed.]
>>
>>  arch/sparc/kernel/prom_common.c |   4 +-
>>  drivers/of/base.c               | 100 ++++++++++++++++++++++------------------
>>  include/linux/of.h              |   2 +-
>>  3 files changed, 59 insertions(+), 47 deletions(-)
> 
> Applied.

This commit is present in next-20130211, and causes a boot failure
(hang) early while booting on Tegra. Reverting just this one commit
solves the issue.

I'll see if I can track down where the issue is. Given the commit
description, I assume there's some new recursive lock issue that snuck
in between the previous fix for them and this commit? Any hints welcome.

One thing I wonder looking at the patch: Most paths use
raw_spin_lock_irqsave() but a few use just raw_spin_lock(). I wonder how
that decision was made?

  parent reply	other threads:[~2013-02-11 19:29 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04 16:05 [PATCH next] OF: convert devtree lock from rw_lock to raw spinlock Paul Gortmaker
2013-02-04 16:05 ` Paul Gortmaker
2013-02-04 16:05 ` Paul Gortmaker
2013-02-04 17:54 ` David Miller
2013-02-04 17:54   ` David Miller
2013-02-06 16:04 ` Rob Herring
2013-02-06 16:04   ` Rob Herring
2013-02-06 20:30 ` [PATCH next v2] " Paul Gortmaker
2013-02-06 20:30   ` Paul Gortmaker
2013-02-06 20:30   ` Paul Gortmaker
2013-02-08 23:09   ` Rob Herring
2013-02-08 23:09     ` Rob Herring
     [not found]     ` <511585A9.9090008-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-02-11 19:29       ` Stephen Warren [this message]
2013-02-11 19:29         ` Stephen Warren
2013-02-11 19:29         ` Stephen Warren
2013-02-11 19:54         ` Rob Herring
2013-02-11 19:54           ` Rob Herring
2013-02-11 22:18           ` Grant Likely
2013-02-11 22:21             ` Rob Herring
2013-02-11 22:21               ` Rob Herring
2013-02-11 22:29               ` Grant Likely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51194685.5010904@wwwdotorg.org \
    --to=swarren-3lzwwm7+weoh9zmkesr00q@public.gmane.org \
    --cc=devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-next-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-rt-users-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org \
    --cc=robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org \
    --cc=sparclinux-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.