linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ntp: Use freezable workqueue for RTC synchronization
@ 2021-01-25 14:30 Geert Uytterhoeven
  2021-02-05 12:43 ` Rafael J. Wysocki
  2021-02-05 17:09 ` [tip: timers/urgent] " tip-bot2 for Geert Uytterhoeven
  0 siblings, 2 replies; 3+ messages in thread
From: Geert Uytterhoeven @ 2021-01-25 14:30 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: Stephen Boyd, Alexandre Belloni, Rafael J . Wysocki, Len Brown,
	Pavel Machek, Wolfram Sang, Viresh Kumar, Tejun Heo,
	Lai Jiangshan, Alessandro Zummo, linux-rtc, linux-pm,
	linux-renesas-soc, linux-kernel, Geert Uytterhoeven

The bug fixed by commit e3fab2f3de081e98 ("ntp: Fix RTC synchronization
on 32-bit platforms") revealed an underlying issue: RTC synchronization
may happen anytime, even while the system is partially suspended.

On systems where the RTC is connected to an I2C bus, the I2C bus
controller may already or still be suspended, triggering a WARNING
during suspend or resume from s2ram:

    WARNING: CPU: 0 PID: 124 at drivers/i2c/i2c-core.h:54 __i2c_transfer+0x634/0x680
    i2c i2c-6: Transfer while suspended
    [...]
    Workqueue: events_power_efficient sync_hw_clock
    [...]
    [<c0738e08>] (__i2c_transfer) from [<c0738eac>] (i2c_transfer+0x58/0xf8)
    [<c0738eac>] (i2c_transfer) from [<c065202c>] (regmap_i2c_read+0x58/0x94)
    [<c065202c>] (regmap_i2c_read) from [<c064de40>] (_regmap_raw_read+0x19c/0x2f4)
    [<c064de40>] (_regmap_raw_read) from [<c064dfdc>] (_regmap_bus_read+0x44/0x68)
    [<c064dfdc>] (_regmap_bus_read) from [<c064ccb4>] (_regmap_read+0x84/0x1a4)
    [<c064ccb4>] (_regmap_read) from [<c064d334>] (_regmap_update_bits+0xa8/0xf4)
    [<c064d334>] (_regmap_update_bits) from [<c064d464>] (_regmap_select_page+0xe4/0x100)
    [<c064d464>] (_regmap_select_page) from [<c064d554>] (_regmap_raw_write_impl+0xd4/0x6c4)
    [<c064d554>] (_regmap_raw_write_impl) from [<c064ec10>] (_regmap_raw_write+0xd8/0x114)
    [<c064ec10>] (_regmap_raw_write) from [<c064eca4>] (regmap_raw_write+0x58/0x7c)
    [<c064eca4>] (regmap_raw_write) from [<c064ede0>] (regmap_bulk_write+0x118/0x13c)
    [<c064ede0>] (regmap_bulk_write) from [<c073660c>] (da9063_rtc_set_time+0x44/0x8c)
    [<c073660c>] (da9063_rtc_set_time) from [<c0734164>] (rtc_set_time+0xc8/0x228)
    [<c0734164>] (rtc_set_time) from [<c02abe78>] (sync_hw_clock+0x128/0x1fc)
    [<c02abe78>] (sync_hw_clock) from [<c023e6a0>] (process_one_work+0x330/0x550)
    [<c023e6a0>] (process_one_work) from [<c023f0a8>] (worker_thread+0x22c/0x2ec)

Fix this race condition by using the freezable instead of the normal
power-efficient workqueue.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 kernel/time/ntp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 54d52fab201d283e..6310328fe398406a 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -502,7 +502,7 @@ static struct hrtimer sync_hrtimer;
 
 static enum hrtimer_restart sync_timer_callback(struct hrtimer *timer)
 {
-	queue_work(system_power_efficient_wq, &sync_work);
+	queue_work(system_freezable_power_efficient_wq, &sync_work);
 
 	return HRTIMER_NORESTART;
 }
@@ -668,7 +668,7 @@ void ntp_notify_cmos_timer(void)
 	 * just a pointless work scheduled.
 	 */
 	if (ntp_synced() && !hrtimer_is_queued(&sync_hrtimer))
-		queue_work(system_power_efficient_wq, &sync_work);
+		queue_work(system_freezable_power_efficient_wq, &sync_work);
 }
 
 static void __init ntp_init_cmos_sync(void)
-- 
2.25.1


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

* Re: [PATCH] ntp: Use freezable workqueue for RTC synchronization
  2021-01-25 14:30 [PATCH] ntp: Use freezable workqueue for RTC synchronization Geert Uytterhoeven
@ 2021-02-05 12:43 ` Rafael J. Wysocki
  2021-02-05 17:09 ` [tip: timers/urgent] " tip-bot2 for Geert Uytterhoeven
  1 sibling, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2021-02-05 12:43 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: John Stultz, Thomas Gleixner, Stephen Boyd, Alexandre Belloni,
	Rafael J . Wysocki, Len Brown, Pavel Machek, Wolfram Sang,
	Viresh Kumar, Tejun Heo, Lai Jiangshan, Alessandro Zummo,
	linux-rtc, Linux PM, Linux-Renesas, Linux Kernel Mailing List

On Tue, Jan 26, 2021 at 6:48 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> The bug fixed by commit e3fab2f3de081e98 ("ntp: Fix RTC synchronization
> on 32-bit platforms") revealed an underlying issue: RTC synchronization
> may happen anytime, even while the system is partially suspended.
>
> On systems where the RTC is connected to an I2C bus, the I2C bus
> controller may already or still be suspended, triggering a WARNING
> during suspend or resume from s2ram:
>
>     WARNING: CPU: 0 PID: 124 at drivers/i2c/i2c-core.h:54 __i2c_transfer+0x634/0x680
>     i2c i2c-6: Transfer while suspended
>     [...]
>     Workqueue: events_power_efficient sync_hw_clock
>     [...]
>     [<c0738e08>] (__i2c_transfer) from [<c0738eac>] (i2c_transfer+0x58/0xf8)
>     [<c0738eac>] (i2c_transfer) from [<c065202c>] (regmap_i2c_read+0x58/0x94)
>     [<c065202c>] (regmap_i2c_read) from [<c064de40>] (_regmap_raw_read+0x19c/0x2f4)
>     [<c064de40>] (_regmap_raw_read) from [<c064dfdc>] (_regmap_bus_read+0x44/0x68)
>     [<c064dfdc>] (_regmap_bus_read) from [<c064ccb4>] (_regmap_read+0x84/0x1a4)
>     [<c064ccb4>] (_regmap_read) from [<c064d334>] (_regmap_update_bits+0xa8/0xf4)
>     [<c064d334>] (_regmap_update_bits) from [<c064d464>] (_regmap_select_page+0xe4/0x100)
>     [<c064d464>] (_regmap_select_page) from [<c064d554>] (_regmap_raw_write_impl+0xd4/0x6c4)
>     [<c064d554>] (_regmap_raw_write_impl) from [<c064ec10>] (_regmap_raw_write+0xd8/0x114)
>     [<c064ec10>] (_regmap_raw_write) from [<c064eca4>] (regmap_raw_write+0x58/0x7c)
>     [<c064eca4>] (regmap_raw_write) from [<c064ede0>] (regmap_bulk_write+0x118/0x13c)
>     [<c064ede0>] (regmap_bulk_write) from [<c073660c>] (da9063_rtc_set_time+0x44/0x8c)
>     [<c073660c>] (da9063_rtc_set_time) from [<c0734164>] (rtc_set_time+0xc8/0x228)
>     [<c0734164>] (rtc_set_time) from [<c02abe78>] (sync_hw_clock+0x128/0x1fc)
>     [<c02abe78>] (sync_hw_clock) from [<c023e6a0>] (process_one_work+0x330/0x550)
>     [<c023e6a0>] (process_one_work) from [<c023f0a8>] (worker_thread+0x22c/0x2ec)
>
> Fix this race condition by using the freezable instead of the normal
> power-efficient workqueue.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

LGTM

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

> ---
>  kernel/time/ntp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
> index 54d52fab201d283e..6310328fe398406a 100644
> --- a/kernel/time/ntp.c
> +++ b/kernel/time/ntp.c
> @@ -502,7 +502,7 @@ static struct hrtimer sync_hrtimer;
>
>  static enum hrtimer_restart sync_timer_callback(struct hrtimer *timer)
>  {
> -       queue_work(system_power_efficient_wq, &sync_work);
> +       queue_work(system_freezable_power_efficient_wq, &sync_work);
>
>         return HRTIMER_NORESTART;
>  }
> @@ -668,7 +668,7 @@ void ntp_notify_cmos_timer(void)
>          * just a pointless work scheduled.
>          */
>         if (ntp_synced() && !hrtimer_is_queued(&sync_hrtimer))
> -               queue_work(system_power_efficient_wq, &sync_work);
> +               queue_work(system_freezable_power_efficient_wq, &sync_work);
>  }
>
>  static void __init ntp_init_cmos_sync(void)
> --
> 2.25.1
>

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

* [tip: timers/urgent] ntp: Use freezable workqueue for RTC synchronization
  2021-01-25 14:30 [PATCH] ntp: Use freezable workqueue for RTC synchronization Geert Uytterhoeven
  2021-02-05 12:43 ` Rafael J. Wysocki
@ 2021-02-05 17:09 ` tip-bot2 for Geert Uytterhoeven
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot2 for Geert Uytterhoeven @ 2021-02-05 17:09 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Geert Uytterhoeven, Thomas Gleixner, Rafael J. Wysocki, x86,
	linux-kernel

The following commit has been merged into the timers/urgent branch of tip:

Commit-ID:     24c242ec7abb3d21fa0b1da6bb251521dc1717b5
Gitweb:        https://git.kernel.org/tip/24c242ec7abb3d21fa0b1da6bb251521dc1717b5
Author:        Geert Uytterhoeven <geert+renesas@glider.be>
AuthorDate:    Mon, 25 Jan 2021 15:30:39 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 05 Feb 2021 18:03:13 +01:00

ntp: Use freezable workqueue for RTC synchronization

The bug fixed by commit e3fab2f3de081e98 ("ntp: Fix RTC synchronization on
32-bit platforms") revealed an underlying issue: RTC synchronization may
happen anytime, even while the system is partially suspended.

On systems where the RTC is connected to an I2C bus, the I2C bus controller
may already or still be suspended, triggering a WARNING during suspend or
resume from s2ram:

    WARNING: CPU: 0 PID: 124 at drivers/i2c/i2c-core.h:54 __i2c_transfer+0x634/0x680
    i2c i2c-6: Transfer while suspended
    [...]
    Workqueue: events_power_efficient sync_hw_clock
    [...]
      (__i2c_transfer)
      (i2c_transfer)
      (regmap_i2c_read)
      ...
      (da9063_rtc_set_time)
      (rtc_set_time)
      (sync_hw_clock)
      (process_one_work)

Fix this race condition by using the freezable instead of the normal
power-efficient workqueue.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
Link: https://lore.kernel.org/r/20210125143039.1051912-1-geert+renesas@glider.be

---
 kernel/time/ntp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 87389b9..5247afd 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -502,7 +502,7 @@ static struct hrtimer sync_hrtimer;
 
 static enum hrtimer_restart sync_timer_callback(struct hrtimer *timer)
 {
-	queue_work(system_power_efficient_wq, &sync_work);
+	queue_work(system_freezable_power_efficient_wq, &sync_work);
 
 	return HRTIMER_NORESTART;
 }
@@ -668,7 +668,7 @@ void ntp_notify_cmos_timer(void)
 	 * just a pointless work scheduled.
 	 */
 	if (ntp_synced() && !hrtimer_is_queued(&sync_hrtimer))
-		queue_work(system_power_efficient_wq, &sync_work);
+		queue_work(system_freezable_power_efficient_wq, &sync_work);
 }
 
 static void __init ntp_init_cmos_sync(void)

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

end of thread, other threads:[~2021-02-06  0:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-25 14:30 [PATCH] ntp: Use freezable workqueue for RTC synchronization Geert Uytterhoeven
2021-02-05 12:43 ` Rafael J. Wysocki
2021-02-05 17:09 ` [tip: timers/urgent] " tip-bot2 for Geert Uytterhoeven

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