From: Vincent Guittot <vincent.guittot@linaro.org> To: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org, rjw@rjwysocki.net, ulf.hansson@linaro.org, biju.das@bp.renesas.com Cc: geert@linux-m68k.org, linux-renesas-soc@vger.kernel.org, Vincent Guittot <vincent.guittot@linaro.org> Subject: [PATCH v2 ] PM-runtime: fix deadlock with ktime Date: Wed, 30 Jan 2019 12:16:24 +0100 [thread overview] Message-ID: <1548846984-2044-1-git-send-email-vincent.guittot@linaro.org> (raw) A deadlock has been seen when swicthing clocksources which use PM runtime. The call path is: change_clocksource ... write_seqcount_begin ... timekeeping_update ... sh_cmt_clocksource_enable ... rpm_resume pm_runtime_mark_last_busy ktime_get do read_seqcount_begin while read_seqcount_retry .... write_seqcount_end Although we should be safe because we haven't yet changed the clocksource at that time, we can't because of seqcount protection. Use ktime_get_mono_fast_ns() instead which is lock safe for such case With ktime_get_mono_fast_ns, the timestamp is not guaranteed to be monotonic across an update and as a result can goes backward. According to update_fast_timekeeper() description: "In the worst case, this can result is a slightly wrong timestamp (a few nanoseconds)". For PM runtime autosuspend, this means only that the suspend decision can be slightly sub optimal. Fixes: 8234f6734c5d ("PM-runtime: Switch autosuspend over to using hrtimers") Reported-by: Biju Das <biju.das@bp.renesas.com> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> --- - v2: Updated commit message to explain the impact of using ktime_get_mono_fast_ns() drivers/base/power/runtime.c | 10 +++++----- include/linux/pm_runtime.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 457be03..708a13f 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -130,7 +130,7 @@ u64 pm_runtime_autosuspend_expiration(struct device *dev) { int autosuspend_delay; u64 last_busy, expires = 0; - u64 now = ktime_to_ns(ktime_get()); + u64 now = ktime_get_mono_fast_ns(); if (!dev->power.use_autosuspend) goto out; @@ -909,7 +909,7 @@ static enum hrtimer_restart pm_suspend_timer_fn(struct hrtimer *timer) * If 'expires' is after the current time, we've been called * too early. */ - if (expires > 0 && expires < ktime_to_ns(ktime_get())) { + if (expires > 0 && expires < ktime_get_mono_fast_ns()) { dev->power.timer_expires = 0; rpm_suspend(dev, dev->power.timer_autosuspends ? (RPM_ASYNC | RPM_AUTO) : RPM_ASYNC); @@ -928,7 +928,7 @@ static enum hrtimer_restart pm_suspend_timer_fn(struct hrtimer *timer) int pm_schedule_suspend(struct device *dev, unsigned int delay) { unsigned long flags; - ktime_t expires; + u64 expires; int retval; spin_lock_irqsave(&dev->power.lock, flags); @@ -945,8 +945,8 @@ int pm_schedule_suspend(struct device *dev, unsigned int delay) /* Other scheduled or pending requests need to be canceled. */ pm_runtime_cancel_pending(dev); - expires = ktime_add(ktime_get(), ms_to_ktime(delay)); - dev->power.timer_expires = ktime_to_ns(expires); + expires = ktime_get_mono_fast_ns() + (u64)delay * NSEC_PER_MSEC); + dev->power.timer_expires = expires; dev->power.timer_autosuspends = 0; hrtimer_start(&dev->power.suspend_timer, expires, HRTIMER_MODE_ABS); diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 54af4ee..fed5be7 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -105,7 +105,7 @@ static inline bool pm_runtime_callbacks_present(struct device *dev) static inline void pm_runtime_mark_last_busy(struct device *dev) { - WRITE_ONCE(dev->power.last_busy, ktime_to_ns(ktime_get())); + WRITE_ONCE(dev->power.last_busy, ktime_get_mono_fast_ns()); } static inline bool pm_runtime_is_irq_safe(struct device *dev) -- 2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: Vincent Guittot <vincent.guittot@linaro.org> To: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org, rjw@rjwysocki.net, ulf.hansson@linaro.org, biju.das@bp.renesas.com Cc: linux-renesas-soc@vger.kernel.org, Vincent Guittot <vincent.guittot@linaro.org>, geert@linux-m68k.org Subject: [PATCH v2 ] PM-runtime: fix deadlock with ktime Date: Wed, 30 Jan 2019 12:16:24 +0100 [thread overview] Message-ID: <1548846984-2044-1-git-send-email-vincent.guittot@linaro.org> (raw) A deadlock has been seen when swicthing clocksources which use PM runtime. The call path is: change_clocksource ... write_seqcount_begin ... timekeeping_update ... sh_cmt_clocksource_enable ... rpm_resume pm_runtime_mark_last_busy ktime_get do read_seqcount_begin while read_seqcount_retry .... write_seqcount_end Although we should be safe because we haven't yet changed the clocksource at that time, we can't because of seqcount protection. Use ktime_get_mono_fast_ns() instead which is lock safe for such case With ktime_get_mono_fast_ns, the timestamp is not guaranteed to be monotonic across an update and as a result can goes backward. According to update_fast_timekeeper() description: "In the worst case, this can result is a slightly wrong timestamp (a few nanoseconds)". For PM runtime autosuspend, this means only that the suspend decision can be slightly sub optimal. Fixes: 8234f6734c5d ("PM-runtime: Switch autosuspend over to using hrtimers") Reported-by: Biju Das <biju.das@bp.renesas.com> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> --- - v2: Updated commit message to explain the impact of using ktime_get_mono_fast_ns() drivers/base/power/runtime.c | 10 +++++----- include/linux/pm_runtime.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 457be03..708a13f 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -130,7 +130,7 @@ u64 pm_runtime_autosuspend_expiration(struct device *dev) { int autosuspend_delay; u64 last_busy, expires = 0; - u64 now = ktime_to_ns(ktime_get()); + u64 now = ktime_get_mono_fast_ns(); if (!dev->power.use_autosuspend) goto out; @@ -909,7 +909,7 @@ static enum hrtimer_restart pm_suspend_timer_fn(struct hrtimer *timer) * If 'expires' is after the current time, we've been called * too early. */ - if (expires > 0 && expires < ktime_to_ns(ktime_get())) { + if (expires > 0 && expires < ktime_get_mono_fast_ns()) { dev->power.timer_expires = 0; rpm_suspend(dev, dev->power.timer_autosuspends ? (RPM_ASYNC | RPM_AUTO) : RPM_ASYNC); @@ -928,7 +928,7 @@ static enum hrtimer_restart pm_suspend_timer_fn(struct hrtimer *timer) int pm_schedule_suspend(struct device *dev, unsigned int delay) { unsigned long flags; - ktime_t expires; + u64 expires; int retval; spin_lock_irqsave(&dev->power.lock, flags); @@ -945,8 +945,8 @@ int pm_schedule_suspend(struct device *dev, unsigned int delay) /* Other scheduled or pending requests need to be canceled. */ pm_runtime_cancel_pending(dev); - expires = ktime_add(ktime_get(), ms_to_ktime(delay)); - dev->power.timer_expires = ktime_to_ns(expires); + expires = ktime_get_mono_fast_ns() + (u64)delay * NSEC_PER_MSEC); + dev->power.timer_expires = expires; dev->power.timer_autosuspends = 0; hrtimer_start(&dev->power.suspend_timer, expires, HRTIMER_MODE_ABS); diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 54af4ee..fed5be7 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -105,7 +105,7 @@ static inline bool pm_runtime_callbacks_present(struct device *dev) static inline void pm_runtime_mark_last_busy(struct device *dev) { - WRITE_ONCE(dev->power.last_busy, ktime_to_ns(ktime_get())); + WRITE_ONCE(dev->power.last_busy, ktime_get_mono_fast_ns()); } static inline bool pm_runtime_is_irq_safe(struct device *dev) -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2019-01-30 11:16 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-30 11:16 Vincent Guittot [this message] 2019-01-30 11:16 ` [PATCH v2 ] PM-runtime: fix deadlock with ktime Vincent Guittot 2019-01-30 12:14 ` Ulf Hansson 2019-01-30 12:14 ` Ulf Hansson 2019-01-30 12:14 ` Ulf Hansson 2019-02-05 13:24 ` Geert Uytterhoeven 2019-02-05 13:24 ` Geert Uytterhoeven 2019-02-05 13:40 ` Ulf Hansson 2019-02-05 13:40 ` Ulf Hansson 2019-01-30 13:06 ` Rafael J. Wysocki 2019-01-30 13:06 ` Rafael J. Wysocki 2019-01-30 13:18 ` Vincent Guittot 2019-01-30 13:18 ` Vincent Guittot 2019-01-30 19:39 ` Ladislav Michl 2019-01-30 19:39 ` Ladislav Michl 2019-01-30 17:26 ` [PATCH v3] " Vincent Guittot 2019-01-30 17:26 ` Vincent Guittot 2019-01-30 21:53 ` Rafael J. Wysocki 2019-01-30 21:53 ` Rafael J. Wysocki 2019-01-30 21:53 ` Rafael J. Wysocki 2019-02-01 15:02 ` Biju Das 2019-02-01 15:02 ` Biju Das 2019-02-01 15:28 ` Vincent Guittot 2019-02-01 15:28 ` Vincent Guittot 2019-02-01 15:44 ` Biju Das 2019-02-01 15:44 ` Biju Das 2019-02-01 15:48 ` Vincent Guittot 2019-02-01 15:48 ` Vincent Guittot 2019-02-01 15:52 ` Vincent Guittot 2019-02-01 15:52 ` Vincent Guittot 2019-02-04 9:29 ` Biju Das 2019-02-04 9:29 ` Biju Das 2019-02-01 15:45 ` Vincent Guittot 2019-02-01 15:45 ` Vincent Guittot 2019-01-30 21:32 ` [PATCH v2 ] " Ladislav Michl 2019-01-30 21:32 ` Ladislav Michl
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=1548846984-2044-1-git-send-email-vincent.guittot@linaro.org \ --to=vincent.guittot@linaro.org \ --cc=biju.das@bp.renesas.com \ --cc=geert@linux-m68k.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=rjw@rjwysocki.net \ --cc=ulf.hansson@linaro.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: linkBe 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.