linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* PM / wakeirq: Convert to SRCU
@ 2017-06-24  9:56 Thomas Gleixner
  2017-06-24 12:02 ` Rafael J. Wysocki
  2017-06-24 13:53 ` Paul E. McKenney
  0 siblings, 2 replies; 12+ messages in thread
From: Thomas Gleixner @ 2017-06-24  9:56 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Brian Norris, Heiko Stuebner, Linus Walleij, linux-rockchip,
	Julia Cartwright, LKML, linux-gpio, John Keeping, linux-pm,
	Doug Anderson, Peter Zijlstra, Tony Lindgren, Rafael J. Wysocki

The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
breaks the irq bus locking infrastructure, which allows sleeping functions
to be called so interrupt controllers behind slow busses, e.g. i2c, can be
handled.

The wakeirq functions hold rcu_read_lock and call into irq functions, which
in case of interrupts using the irq bus locking will trigger a
might_sleep() splat.

Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.

Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
Reported-by: Brian Norris <briannorris@chromium.org>
Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org

---
 drivers/base/power/wakeup.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -60,6 +60,8 @@ static LIST_HEAD(wakeup_sources);
 
 static DECLARE_WAIT_QUEUE_HEAD(wakeup_count_wait_queue);
 
+static struct srcu_struct wakeup_srcu;
+
 static struct wakeup_source deleted_ws = {
 	.name = "deleted",
 	.lock =  __SPIN_LOCK_UNLOCKED(deleted_ws.lock),
@@ -198,7 +200,7 @@ void wakeup_source_remove(struct wakeup_
 	spin_lock_irqsave(&events_lock, flags);
 	list_del_rcu(&ws->entry);
 	spin_unlock_irqrestore(&events_lock, flags);
-	synchronize_rcu();
+	synchronize_srcu(&wakeup_srcu);
 }
 EXPORT_SYMBOL_GPL(wakeup_source_remove);
 
@@ -804,10 +806,10 @@ EXPORT_SYMBOL_GPL(pm_wakeup_dev_event);
 void pm_print_active_wakeup_sources(void)
 {
 	struct wakeup_source *ws;
-	int active = 0;
+	int srcuidx, active = 0;
 	struct wakeup_source *last_activity_ws = NULL;
 
-	rcu_read_lock();
+	srcuidx = srcu_read_lock(&wakeup_srcu);
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
 		if (ws->active) {
 			pr_debug("active wakeup source: %s\n", ws->name);
@@ -823,7 +825,7 @@ void pm_print_active_wakeup_sources(void
 	if (!active && last_activity_ws)
 		pr_debug("last active wakeup source: %s\n",
 			last_activity_ws->name);
-	rcu_read_unlock();
+	srcu_read_unlock(&wakeup_srcu, srcuidx);
 }
 EXPORT_SYMBOL_GPL(pm_print_active_wakeup_sources);
 
@@ -950,8 +952,9 @@ void pm_wakep_autosleep_enabled(bool set
 {
 	struct wakeup_source *ws;
 	ktime_t now = ktime_get();
+	int srcuidx;
 
-	rcu_read_lock();
+	srcuidx = srcu_read_lock(&wakeup_srcu);
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
 		spin_lock_irq(&ws->lock);
 		if (ws->autosleep_enabled != set) {
@@ -965,7 +968,7 @@ void pm_wakep_autosleep_enabled(bool set
 		}
 		spin_unlock_irq(&ws->lock);
 	}
-	rcu_read_unlock();
+	srcu_read_unlock(&wakeup_srcu, srcuidx);
 }
 #endif /* CONFIG_PM_AUTOSLEEP */
 
@@ -1026,15 +1029,16 @@ static int print_wakeup_source_stats(str
 static int wakeup_sources_stats_show(struct seq_file *m, void *unused)
 {
 	struct wakeup_source *ws;
+	int srcuidx;
 
 	seq_puts(m, "name\t\tactive_count\tevent_count\twakeup_count\t"
 		"expire_count\tactive_since\ttotal_time\tmax_time\t"
 		"last_change\tprevent_suspend_time\n");
 
-	rcu_read_lock();
+	srcuidx = srcu_read_lock(&wakeup_srcu);
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
 		print_wakeup_source_stats(m, ws);
-	rcu_read_unlock();
+	srcu_read_unlock(&wakeup_srcu, srcuidx);
 
 	print_wakeup_source_stats(m, &deleted_ws);
 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PM / wakeirq: Convert to SRCU
  2017-06-24  9:56 PM / wakeirq: Convert to SRCU Thomas Gleixner
@ 2017-06-24 12:02 ` Rafael J. Wysocki
  2017-06-24 12:13   ` Thomas Gleixner
  2017-06-24 13:53 ` Paul E. McKenney
  1 sibling, 1 reply; 12+ messages in thread
From: Rafael J. Wysocki @ 2017-06-24 12:02 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Paul E. McKenney, Brian Norris, Heiko Stuebner, Linus Walleij,
	linux-rockchip, Julia Cartwright, LKML, linux-gpio, John Keeping,
	Linux PM, Doug Anderson, Peter Zijlstra, Tony Lindgren,
	Rafael J. Wysocki

On Sat, Jun 24, 2017 at 11:56 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
> breaks the irq bus locking infrastructure, which allows sleeping functions
> to be called so interrupt controllers behind slow busses, e.g. i2c, can be
> handled.
>
> The wakeirq functions hold rcu_read_lock and call into irq functions, which
> in case of interrupts using the irq bus locking will trigger a
> might_sleep() splat.
>
> Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.
>
> Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> Reported-by: Brian Norris <briannorris@chromium.org>
> Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: stable@vger.kernel.org

OK

I guess it would be good to get this into 4.12?

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PM / wakeirq: Convert to SRCU
  2017-06-24 12:02 ` Rafael J. Wysocki
@ 2017-06-24 12:13   ` Thomas Gleixner
  2017-06-24 12:30     ` Rafael J. Wysocki
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2017-06-24 12:13 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Paul E. McKenney, Brian Norris, Heiko Stuebner, Linus Walleij,
	linux-rockchip, Julia Cartwright, LKML, linux-gpio, John Keeping,
	Linux PM, Doug Anderson, Peter Zijlstra, Tony Lindgren,
	Rafael J. Wysocki

On Sat, 24 Jun 2017, Rafael J. Wysocki wrote:
> On Sat, Jun 24, 2017 at 11:56 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
> > breaks the irq bus locking infrastructure, which allows sleeping functions
> > to be called so interrupt controllers behind slow busses, e.g. i2c, can be
> > handled.
> >
> > The wakeirq functions hold rcu_read_lock and call into irq functions, which
> > in case of interrupts using the irq bus locking will trigger a
> > might_sleep() splat.
> >
> > Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.
> >
> > Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> > Reported-by: Brian Norris <briannorris@chromium.org>
> > Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Cc: stable@vger.kernel.org
> 
> OK
> 
> I guess it would be good to get this into 4.12?

I think so, but it probably wants some testing and breeding in next.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PM / wakeirq: Convert to SRCU
  2017-06-24 12:13   ` Thomas Gleixner
@ 2017-06-24 12:30     ` Rafael J. Wysocki
  0 siblings, 0 replies; 12+ messages in thread
From: Rafael J. Wysocki @ 2017-06-24 12:30 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Rafael J. Wysocki, Paul E. McKenney, Brian Norris,
	Heiko Stuebner, Linus Walleij, linux-rockchip, Julia Cartwright,
	LKML, linux-gpio, John Keeping, Linux PM, Doug Anderson,
	Peter Zijlstra, Tony Lindgren

On Saturday, June 24, 2017 02:13:35 PM Thomas Gleixner wrote:
> On Sat, 24 Jun 2017, Rafael J. Wysocki wrote:
> > On Sat, Jun 24, 2017 at 11:56 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > > The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
> > > breaks the irq bus locking infrastructure, which allows sleeping functions
> > > to be called so interrupt controllers behind slow busses, e.g. i2c, can be
> > > handled.
> > >
> > > The wakeirq functions hold rcu_read_lock and call into irq functions, which
> > > in case of interrupts using the irq bus locking will trigger a
> > > might_sleep() splat.
> > >
> > > Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.
> > >
> > > Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> > > Reported-by: Brian Norris <briannorris@chromium.org>
> > > Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: stable@vger.kernel.org
> > 
> > OK
> > 
> > I guess it would be good to get this into 4.12?
> 
> I think so, but it probably wants some testing and breeding in next.

Right.

So here's a deal: if there is an -rc8, I can push this to Linus before the
final 4.12, but otherwise it will go into 4.13-rc during the merge window.

The -stable link will still be there, so that shouldn't be a big issue.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PM / wakeirq: Convert to SRCU
  2017-06-24  9:56 PM / wakeirq: Convert to SRCU Thomas Gleixner
  2017-06-24 12:02 ` Rafael J. Wysocki
@ 2017-06-24 13:53 ` Paul E. McKenney
  2017-06-25  9:11   ` Thomas Gleixner
  1 sibling, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2017-06-24 13:53 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Brian Norris, Heiko Stuebner, Linus Walleij, linux-rockchip,
	Julia Cartwright, LKML, linux-gpio, John Keeping, linux-pm,
	Doug Anderson, Peter Zijlstra, Tony Lindgren, Rafael J. Wysocki

On Sat, Jun 24, 2017 at 11:56:11AM +0200, Thomas Gleixner wrote:
> The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
> breaks the irq bus locking infrastructure, which allows sleeping functions
> to be called so interrupt controllers behind slow busses, e.g. i2c, can be
> handled.
> 
> The wakeirq functions hold rcu_read_lock and call into irq functions, which
> in case of interrupts using the irq bus locking will trigger a
> might_sleep() splat.
> 
> Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.
> 
> Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> Reported-by: Brian Norris <briannorris@chromium.org>
> Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: stable@vger.kernel.org

Looks plausible.  One suggestion on initialization below, and a couple
of questions about uses of RCU that this patch does not convert to SRCU.

							Thanx, Paul

> ---
>  drivers/base/power/wakeup.c |   20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -60,6 +60,8 @@ static LIST_HEAD(wakeup_sources);
> 
>  static DECLARE_WAIT_QUEUE_HEAD(wakeup_count_wait_queue);
> 
> +static struct srcu_struct wakeup_srcu;

I suggest this to avoid the need for boot-time init_srcu_struct():

DEFINE_STATIC_SRCU(wakeup_srcu);

> +
>  static struct wakeup_source deleted_ws = {
>  	.name = "deleted",
>  	.lock =  __SPIN_LOCK_UNLOCKED(deleted_ws.lock),
> @@ -198,7 +200,7 @@ void wakeup_source_remove(struct wakeup_
>  	spin_lock_irqsave(&events_lock, flags);
>  	list_del_rcu(&ws->entry);
>  	spin_unlock_irqrestore(&events_lock, flags);
> -	synchronize_rcu();
> +	synchronize_srcu(&wakeup_srcu);
>  }
>  EXPORT_SYMBOL_GPL(wakeup_source_remove);

The uses of RCU in device_wakeup_arm_wake_irqs() and
device_wakeup_disarm_wake_irqs() are unrelated and thus do not
need to be converted?  Or am I looking at the wrong version of
the kernel?  (Looking at f65013d655ac ("Merge branch 'for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace")
in Linus's tree.)

If these two functions are making unrelated use of RCU and thus don't
need to be converted to SRCU, might be worth a comment somewhere.

> @@ -804,10 +806,10 @@ EXPORT_SYMBOL_GPL(pm_wakeup_dev_event);
>  void pm_print_active_wakeup_sources(void)
>  {
>  	struct wakeup_source *ws;
> -	int active = 0;
> +	int srcuidx, active = 0;
>  	struct wakeup_source *last_activity_ws = NULL;
> 
> -	rcu_read_lock();
> +	srcuidx = srcu_read_lock(&wakeup_srcu);
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
>  		if (ws->active) {
>  			pr_debug("active wakeup source: %s\n", ws->name);
> @@ -823,7 +825,7 @@ void pm_print_active_wakeup_sources(void
>  	if (!active && last_activity_ws)
>  		pr_debug("last active wakeup source: %s\n",
>  			last_activity_ws->name);
> -	rcu_read_unlock();
> +	srcu_read_unlock(&wakeup_srcu, srcuidx);
>  }
>  EXPORT_SYMBOL_GPL(pm_print_active_wakeup_sources);
> 
> @@ -950,8 +952,9 @@ void pm_wakep_autosleep_enabled(bool set
>  {
>  	struct wakeup_source *ws;
>  	ktime_t now = ktime_get();
> +	int srcuidx;
> 
> -	rcu_read_lock();
> +	srcuidx = srcu_read_lock(&wakeup_srcu);
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
>  		spin_lock_irq(&ws->lock);
>  		if (ws->autosleep_enabled != set) {
> @@ -965,7 +968,7 @@ void pm_wakep_autosleep_enabled(bool set
>  		}
>  		spin_unlock_irq(&ws->lock);
>  	}
> -	rcu_read_unlock();
> +	srcu_read_unlock(&wakeup_srcu, srcuidx);
>  }
>  #endif /* CONFIG_PM_AUTOSLEEP */

Same question for pm_wakep_autosleep_enabled().

> @@ -1026,15 +1029,16 @@ static int print_wakeup_source_stats(str
>  static int wakeup_sources_stats_show(struct seq_file *m, void *unused)
>  {
>  	struct wakeup_source *ws;
> +	int srcuidx;
> 
>  	seq_puts(m, "name\t\tactive_count\tevent_count\twakeup_count\t"
>  		"expire_count\tactive_since\ttotal_time\tmax_time\t"
>  		"last_change\tprevent_suspend_time\n");
> 
> -	rcu_read_lock();
> +	srcuidx = srcu_read_lock(&wakeup_srcu);
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
>  		print_wakeup_source_stats(m, ws);
> -	rcu_read_unlock();
> +	srcu_read_unlock(&wakeup_srcu, srcuidx);
> 
>  	print_wakeup_source_stats(m, &deleted_ws);
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PM / wakeirq: Convert to SRCU
  2017-06-24 13:53 ` Paul E. McKenney
@ 2017-06-25  9:11   ` Thomas Gleixner
  2017-06-25 14:45     ` Paul E. McKenney
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2017-06-25  9:11 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Brian Norris, Heiko Stuebner, Linus Walleij, linux-rockchip,
	Julia Cartwright, LKML, linux-gpio, John Keeping, linux-pm,
	Doug Anderson, Peter Zijlstra, Tony Lindgren, Rafael J. Wysocki

On Sat, 24 Jun 2017, Paul E. McKenney wrote:
> On Sat, Jun 24, 2017 at 11:56:11AM +0200, Thomas Gleixner wrote:
> >  static DECLARE_WAIT_QUEUE_HEAD(wakeup_count_wait_queue);
> > 
> > +static struct srcu_struct wakeup_srcu;
> 
> I suggest this to avoid the need for boot-time init_srcu_struct():
> 
> DEFINE_STATIC_SRCU(wakeup_srcu);

Now I know why I had this nagging feeling, that should stay away from
computers yesterday morning.....

> >  static struct wakeup_source deleted_ws = {
> >  	.name = "deleted",
> >  	.lock =  __SPIN_LOCK_UNLOCKED(deleted_ws.lock),
> > @@ -198,7 +200,7 @@ void wakeup_source_remove(struct wakeup_
> >  	spin_lock_irqsave(&events_lock, flags);
> >  	list_del_rcu(&ws->entry);
> >  	spin_unlock_irqrestore(&events_lock, flags);
> > -	synchronize_rcu();
> > +	synchronize_srcu(&wakeup_srcu);
> >  }
> >  EXPORT_SYMBOL_GPL(wakeup_source_remove);
> 
> The uses of RCU in device_wakeup_arm_wake_irqs() and
> device_wakeup_disarm_wake_irqs() are unrelated and thus do not
> need to be converted?  Or am I looking at the wrong version of
> the kernel?  (Looking at f65013d655ac ("Merge branch 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace")
> in Linus's tree.)

No, you are looking at the result of heat induced brain melt. Will send a
proper one soon.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PM / wakeirq: Convert to SRCU
  2017-06-25  9:11   ` Thomas Gleixner
@ 2017-06-25 14:45     ` Paul E. McKenney
  2017-06-25 17:31       ` [PATCH v2] " Thomas Gleixner
  0 siblings, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2017-06-25 14:45 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Brian Norris, Heiko Stuebner, Linus Walleij, linux-rockchip,
	Julia Cartwright, LKML, linux-gpio, John Keeping, linux-pm,
	Doug Anderson, Peter Zijlstra, Tony Lindgren, Rafael J. Wysocki

On Sun, Jun 25, 2017 at 11:11:57AM +0200, Thomas Gleixner wrote:
> On Sat, 24 Jun 2017, Paul E. McKenney wrote:
> > On Sat, Jun 24, 2017 at 11:56:11AM +0200, Thomas Gleixner wrote:
> > >  static DECLARE_WAIT_QUEUE_HEAD(wakeup_count_wait_queue);
> > > 
> > > +static struct srcu_struct wakeup_srcu;
> > 
> > I suggest this to avoid the need for boot-time init_srcu_struct():
> > 
> > DEFINE_STATIC_SRCU(wakeup_srcu);
> 
> Now I know why I had this nagging feeling, that should stay away from
> computers yesterday morning.....

Believe me, I know that feeling well!

> > >  static struct wakeup_source deleted_ws = {
> > >  	.name = "deleted",
> > >  	.lock =  __SPIN_LOCK_UNLOCKED(deleted_ws.lock),
> > > @@ -198,7 +200,7 @@ void wakeup_source_remove(struct wakeup_
> > >  	spin_lock_irqsave(&events_lock, flags);
> > >  	list_del_rcu(&ws->entry);
> > >  	spin_unlock_irqrestore(&events_lock, flags);
> > > -	synchronize_rcu();
> > > +	synchronize_srcu(&wakeup_srcu);
> > >  }
> > >  EXPORT_SYMBOL_GPL(wakeup_source_remove);
> > 
> > The uses of RCU in device_wakeup_arm_wake_irqs() and
> > device_wakeup_disarm_wake_irqs() are unrelated and thus do not
> > need to be converted?  Or am I looking at the wrong version of
> > the kernel?  (Looking at f65013d655ac ("Merge branch 'for-linus' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace")
> > in Linus's tree.)
> 
> No, you are looking at the result of heat induced brain melt. Will send a
> proper one soon.

And if anything, I know that feeling even better.  Looking forward to
seeing the new patch.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2]  PM / wakeirq: Convert to SRCU
  2017-06-25 14:45     ` Paul E. McKenney
@ 2017-06-25 17:31       ` Thomas Gleixner
  2017-06-25 18:29         ` Paul E. McKenney
  2017-06-26 20:37         ` Brian Norris
  0 siblings, 2 replies; 12+ messages in thread
From: Thomas Gleixner @ 2017-06-25 17:31 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Brian Norris, Heiko Stuebner, Linus Walleij, linux-rockchip,
	Julia Cartwright, LKML, linux-gpio, John Keeping, linux-pm,
	Doug Anderson, Peter Zijlstra, Tony Lindgren, Rafael J. Wysocki

The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
breaks the irq bus locking infrastructure, which is allows sleeping
functions to be called so interrupt controllers behind slow busses,
e.g. i2c, can be handled.

The wakeirq functions hold rcu_read_lock and call into irq functions, which
in case of interrupts using the irq bus locking will trigger a
might_sleep() splat.

Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.

Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
Reported-by: Brian Norris <briannorris@chromium.org>
Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/base/power/wakeup.c |   32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -60,6 +60,8 @@ static LIST_HEAD(wakeup_sources);
 
 static DECLARE_WAIT_QUEUE_HEAD(wakeup_count_wait_queue);
 
+DEFINE_STATIC_SRCU(wakeup_srcu);
+
 static struct wakeup_source deleted_ws = {
 	.name = "deleted",
 	.lock =  __SPIN_LOCK_UNLOCKED(deleted_ws.lock),
@@ -198,7 +200,7 @@ void wakeup_source_remove(struct wakeup_
 	spin_lock_irqsave(&events_lock, flags);
 	list_del_rcu(&ws->entry);
 	spin_unlock_irqrestore(&events_lock, flags);
-	synchronize_rcu();
+	synchronize_srcu(&wakeup_srcu);
 }
 EXPORT_SYMBOL_GPL(wakeup_source_remove);
 
@@ -332,12 +334,12 @@ void device_wakeup_detach_irq(struct dev
 void device_wakeup_arm_wake_irqs(void)
 {
 	struct wakeup_source *ws;
+	int srcuidx;
 
-	rcu_read_lock();
+	srcuidx = srcu_read_lock(&wakeup_srcu);
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
 		dev_pm_arm_wake_irq(ws->wakeirq);
-
-	rcu_read_unlock();
+	srcu_read_unlock(&wakeup_srcu, srcuidx);
 }
 
 /**
@@ -348,12 +350,12 @@ void device_wakeup_arm_wake_irqs(void)
 void device_wakeup_disarm_wake_irqs(void)
 {
 	struct wakeup_source *ws;
+	int srcuidx;
 
-	rcu_read_lock();
+	srcuidx = srcu_read_lock(&wakeup_srcu);
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
 		dev_pm_disarm_wake_irq(ws->wakeirq);
-
-	rcu_read_unlock();
+	srcu_read_unlock(&wakeup_srcu, srcuidx);
 }
 
 /**
@@ -804,10 +806,10 @@ EXPORT_SYMBOL_GPL(pm_wakeup_dev_event);
 void pm_print_active_wakeup_sources(void)
 {
 	struct wakeup_source *ws;
-	int active = 0;
+	int srcuidx, active = 0;
 	struct wakeup_source *last_activity_ws = NULL;
 
-	rcu_read_lock();
+	srcuidx = srcu_read_lock(&wakeup_srcu);
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
 		if (ws->active) {
 			pr_debug("active wakeup source: %s\n", ws->name);
@@ -823,7 +825,7 @@ void pm_print_active_wakeup_sources(void
 	if (!active && last_activity_ws)
 		pr_debug("last active wakeup source: %s\n",
 			last_activity_ws->name);
-	rcu_read_unlock();
+	srcu_read_unlock(&wakeup_srcu, srcuidx);
 }
 EXPORT_SYMBOL_GPL(pm_print_active_wakeup_sources);
 
@@ -950,8 +952,9 @@ void pm_wakep_autosleep_enabled(bool set
 {
 	struct wakeup_source *ws;
 	ktime_t now = ktime_get();
+	int srcuidx;
 
-	rcu_read_lock();
+	srcuidx = srcu_read_lock(&wakeup_srcu);
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
 		spin_lock_irq(&ws->lock);
 		if (ws->autosleep_enabled != set) {
@@ -965,7 +968,7 @@ void pm_wakep_autosleep_enabled(bool set
 		}
 		spin_unlock_irq(&ws->lock);
 	}
-	rcu_read_unlock();
+	srcu_read_unlock(&wakeup_srcu, srcuidx);
 }
 #endif /* CONFIG_PM_AUTOSLEEP */
 
@@ -1026,15 +1029,16 @@ static int print_wakeup_source_stats(str
 static int wakeup_sources_stats_show(struct seq_file *m, void *unused)
 {
 	struct wakeup_source *ws;
+	int srcuidx;
 
 	seq_puts(m, "name\t\tactive_count\tevent_count\twakeup_count\t"
 		"expire_count\tactive_since\ttotal_time\tmax_time\t"
 		"last_change\tprevent_suspend_time\n");
 
-	rcu_read_lock();
+	srcuidx = srcu_read_lock(&wakeup_srcu);
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
 		print_wakeup_source_stats(m, ws);
-	rcu_read_unlock();
+	srcu_read_unlock(&wakeup_srcu, srcuidx);
 
 	print_wakeup_source_stats(m, &deleted_ws);
 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2]  PM / wakeirq: Convert to SRCU
  2017-06-25 17:31       ` [PATCH v2] " Thomas Gleixner
@ 2017-06-25 18:29         ` Paul E. McKenney
  2017-06-26 10:15           ` Tony Lindgren
  2017-06-26 20:37         ` Brian Norris
  1 sibling, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2017-06-25 18:29 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Brian Norris, Heiko Stuebner, Linus Walleij, linux-rockchip,
	Julia Cartwright, LKML, linux-gpio, John Keeping, linux-pm,
	Doug Anderson, Peter Zijlstra, Tony Lindgren, Rafael J. Wysocki

On Sun, Jun 25, 2017 at 07:31:13PM +0200, Thomas Gleixner wrote:
> The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
> breaks the irq bus locking infrastructure, which is allows sleeping
> functions to be called so interrupt controllers behind slow busses,
> e.g. i2c, can be handled.
> 
> The wakeirq functions hold rcu_read_lock and call into irq functions, which
> in case of interrupts using the irq bus locking will trigger a
> might_sleep() splat.
> 
> Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.
> 
> Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> Reported-by: Brian Norris <briannorris@chromium.org>
> Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Looks good to me!

Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> ---
>  drivers/base/power/wakeup.c |   32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -60,6 +60,8 @@ static LIST_HEAD(wakeup_sources);
> 
>  static DECLARE_WAIT_QUEUE_HEAD(wakeup_count_wait_queue);
> 
> +DEFINE_STATIC_SRCU(wakeup_srcu);
> +
>  static struct wakeup_source deleted_ws = {
>  	.name = "deleted",
>  	.lock =  __SPIN_LOCK_UNLOCKED(deleted_ws.lock),
> @@ -198,7 +200,7 @@ void wakeup_source_remove(struct wakeup_
>  	spin_lock_irqsave(&events_lock, flags);
>  	list_del_rcu(&ws->entry);
>  	spin_unlock_irqrestore(&events_lock, flags);
> -	synchronize_rcu();
> +	synchronize_srcu(&wakeup_srcu);
>  }
>  EXPORT_SYMBOL_GPL(wakeup_source_remove);
> 
> @@ -332,12 +334,12 @@ void device_wakeup_detach_irq(struct dev
>  void device_wakeup_arm_wake_irqs(void)
>  {
>  	struct wakeup_source *ws;
> +	int srcuidx;
> 
> -	rcu_read_lock();
> +	srcuidx = srcu_read_lock(&wakeup_srcu);
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
>  		dev_pm_arm_wake_irq(ws->wakeirq);
> -
> -	rcu_read_unlock();
> +	srcu_read_unlock(&wakeup_srcu, srcuidx);
>  }
> 
>  /**
> @@ -348,12 +350,12 @@ void device_wakeup_arm_wake_irqs(void)
>  void device_wakeup_disarm_wake_irqs(void)
>  {
>  	struct wakeup_source *ws;
> +	int srcuidx;
> 
> -	rcu_read_lock();
> +	srcuidx = srcu_read_lock(&wakeup_srcu);
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
>  		dev_pm_disarm_wake_irq(ws->wakeirq);
> -
> -	rcu_read_unlock();
> +	srcu_read_unlock(&wakeup_srcu, srcuidx);
>  }
> 
>  /**
> @@ -804,10 +806,10 @@ EXPORT_SYMBOL_GPL(pm_wakeup_dev_event);
>  void pm_print_active_wakeup_sources(void)
>  {
>  	struct wakeup_source *ws;
> -	int active = 0;
> +	int srcuidx, active = 0;
>  	struct wakeup_source *last_activity_ws = NULL;
> 
> -	rcu_read_lock();
> +	srcuidx = srcu_read_lock(&wakeup_srcu);
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
>  		if (ws->active) {
>  			pr_debug("active wakeup source: %s\n", ws->name);
> @@ -823,7 +825,7 @@ void pm_print_active_wakeup_sources(void
>  	if (!active && last_activity_ws)
>  		pr_debug("last active wakeup source: %s\n",
>  			last_activity_ws->name);
> -	rcu_read_unlock();
> +	srcu_read_unlock(&wakeup_srcu, srcuidx);
>  }
>  EXPORT_SYMBOL_GPL(pm_print_active_wakeup_sources);
> 
> @@ -950,8 +952,9 @@ void pm_wakep_autosleep_enabled(bool set
>  {
>  	struct wakeup_source *ws;
>  	ktime_t now = ktime_get();
> +	int srcuidx;
> 
> -	rcu_read_lock();
> +	srcuidx = srcu_read_lock(&wakeup_srcu);
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
>  		spin_lock_irq(&ws->lock);
>  		if (ws->autosleep_enabled != set) {
> @@ -965,7 +968,7 @@ void pm_wakep_autosleep_enabled(bool set
>  		}
>  		spin_unlock_irq(&ws->lock);
>  	}
> -	rcu_read_unlock();
> +	srcu_read_unlock(&wakeup_srcu, srcuidx);
>  }
>  #endif /* CONFIG_PM_AUTOSLEEP */
> 
> @@ -1026,15 +1029,16 @@ static int print_wakeup_source_stats(str
>  static int wakeup_sources_stats_show(struct seq_file *m, void *unused)
>  {
>  	struct wakeup_source *ws;
> +	int srcuidx;
> 
>  	seq_puts(m, "name\t\tactive_count\tevent_count\twakeup_count\t"
>  		"expire_count\tactive_since\ttotal_time\tmax_time\t"
>  		"last_change\tprevent_suspend_time\n");
> 
> -	rcu_read_lock();
> +	srcuidx = srcu_read_lock(&wakeup_srcu);
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
>  		print_wakeup_source_stats(m, ws);
> -	rcu_read_unlock();
> +	srcu_read_unlock(&wakeup_srcu, srcuidx);
> 
>  	print_wakeup_source_stats(m, &deleted_ws);
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2]  PM / wakeirq: Convert to SRCU
  2017-06-25 18:29         ` Paul E. McKenney
@ 2017-06-26 10:15           ` Tony Lindgren
  0 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2017-06-26 10:15 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Thomas Gleixner, Brian Norris, Heiko Stuebner, Linus Walleij,
	linux-rockchip, Julia Cartwright, LKML, linux-gpio, John Keeping,
	linux-pm, Doug Anderson, Peter Zijlstra, Rafael J. Wysocki

* Paul E. McKenney <paulmck@linux.vnet.ibm.com> [170625 11:29]:
> On Sun, Jun 25, 2017 at 07:31:13PM +0200, Thomas Gleixner wrote:
> > The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
> > breaks the irq bus locking infrastructure, which is allows sleeping
> > functions to be called so interrupt controllers behind slow busses,
> > e.g. i2c, can be handled.
> > 
> > The wakeirq functions hold rcu_read_lock and call into irq functions, which
> > in case of interrupts using the irq bus locking will trigger a
> > might_sleep() splat.
> > 
> > Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.
> > 
> > Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> > Reported-by: Brian Norris <briannorris@chromium.org>
> > Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> Looks good to me!
> 
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

And still works for me!

Tested-by: Tony Lindgren <tony@atomide.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2]  PM / wakeirq: Convert to SRCU
  2017-06-25 17:31       ` [PATCH v2] " Thomas Gleixner
  2017-06-25 18:29         ` Paul E. McKenney
@ 2017-06-26 20:37         ` Brian Norris
  2017-06-28 20:54           ` Rafael J. Wysocki
  1 sibling, 1 reply; 12+ messages in thread
From: Brian Norris @ 2017-06-26 20:37 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Paul E. McKenney, Heiko Stuebner, Linus Walleij, linux-rockchip,
	Julia Cartwright, LKML, linux-gpio, John Keeping, linux-pm,
	Doug Anderson, Peter Zijlstra, Tony Lindgren, Rafael J. Wysocki

On Sun, Jun 25, 2017 at 07:31:13PM +0200, Thomas Gleixner wrote:
> The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
> breaks the irq bus locking infrastructure, which is allows sleeping
> functions to be called so interrupt controllers behind slow busses,
> e.g. i2c, can be handled.
> 
> The wakeirq functions hold rcu_read_lock and call into irq functions, which
> in case of interrupts using the irq bus locking will trigger a
> might_sleep() splat.
> 
> Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.
> 
> Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> Reported-by: Brian Norris <briannorris@chromium.org>
> Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  drivers/base/power/wakeup.c |   32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)

This resolves the warnings I've seen, for sure. And I don't seem to have
any new suspend/resume problems, so:

Tested-by: Brian Norris <briannorris@chromium.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2]  PM / wakeirq: Convert to SRCU
  2017-06-26 20:37         ` Brian Norris
@ 2017-06-28 20:54           ` Rafael J. Wysocki
  0 siblings, 0 replies; 12+ messages in thread
From: Rafael J. Wysocki @ 2017-06-28 20:54 UTC (permalink / raw)
  To: Brian Norris, Thomas Gleixner
  Cc: Paul E. McKenney, Heiko Stuebner, Linus Walleij, linux-rockchip,
	Julia Cartwright, LKML, linux-gpio, John Keeping, linux-pm,
	Doug Anderson, Peter Zijlstra, Tony Lindgren

On Monday, June 26, 2017 01:37:11 PM Brian Norris wrote:
> On Sun, Jun 25, 2017 at 07:31:13PM +0200, Thomas Gleixner wrote:
> > The wakeirq infrastructure uses RCU to protect the list of wakeirqs. That
> > breaks the irq bus locking infrastructure, which is allows sleeping
> > functions to be called so interrupt controllers behind slow busses,
> > e.g. i2c, can be handled.
> > 
> > The wakeirq functions hold rcu_read_lock and call into irq functions, which
> > in case of interrupts using the irq bus locking will trigger a
> > might_sleep() splat.
> > 
> > Convert the wakeirq infrastructure to Sleepable RCU and unbreak it.
> > 
> > Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> > Reported-by: Brian Norris <briannorris@chromium.org>
> > Suggested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > ---
> >  drivers/base/power/wakeup.c |   32 ++++++++++++++++++--------------
> >  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> This resolves the warnings I've seen, for sure. And I don't seem to have
> any new suspend/resume problems, so:
> 
> Tested-by: Brian Norris <briannorris@chromium.org>

Applied with all tags, thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-06-28 21:02 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-24  9:56 PM / wakeirq: Convert to SRCU Thomas Gleixner
2017-06-24 12:02 ` Rafael J. Wysocki
2017-06-24 12:13   ` Thomas Gleixner
2017-06-24 12:30     ` Rafael J. Wysocki
2017-06-24 13:53 ` Paul E. McKenney
2017-06-25  9:11   ` Thomas Gleixner
2017-06-25 14:45     ` Paul E. McKenney
2017-06-25 17:31       ` [PATCH v2] " Thomas Gleixner
2017-06-25 18:29         ` Paul E. McKenney
2017-06-26 10:15           ` Tony Lindgren
2017-06-26 20:37         ` Brian Norris
2017-06-28 20:54           ` Rafael J. Wysocki

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).