| Message ID | 20210125143039.1051912-1-geert+renesas@glider.be |
|---|---|
| State | Accepted |
| Commit | 24c242ec7abb3d21fa0b1da6bb251521dc1717b5 |
| Headers | show |
| Series | ntp: Use freezable workqueue for RTC synchronization | expand |
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 >
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)
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(-)