From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0A3EC64ED8 for ; Fri, 24 Feb 2023 10:34:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230360AbjBXKe1 (ORCPT ); Fri, 24 Feb 2023 05:34:27 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58102 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229925AbjBXKdr (ORCPT ); Fri, 24 Feb 2023 05:33:47 -0500 Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 45BF916307 for ; Fri, 24 Feb 2023 02:32:24 -0800 (PST) Received: by mail-ed1-x52b.google.com with SMTP id da10so54241618edb.3 for ; Fri, 24 Feb 2023 02:32:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amarulasolutions.com; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=VgMFdC6ibbZOtd9fIoLXBHhH0VUtRIb+6ju7RO2fD7Y=; b=PdrtKogBI4MK7bvpvoBxKz02S1hTzPtvSVVy6fz+GtgRHpJDCqMDbIzG7BGlRI0TgO iZbrpC8ozYXSvaa0OTn56b7xgSqmn/kcrJxGZ3gwvOYhVDdX1yFr/4FfCnR9yC7m77oq 0h8x35WPbgkeImLdke1XZkfKwrJTGmVzGHKc8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=VgMFdC6ibbZOtd9fIoLXBHhH0VUtRIb+6ju7RO2fD7Y=; b=McmyEg6sJMsyH9Kk0y49XTNSCFwHkybhTghkhuCptpSJPXYARSvycuh71vfnxBuH3x awObfboR/qr9KJZ086D0m0UiUy0MtlIQR/4JwdDNvHSmwZHPqskSaTinxW5UCtL03yh8 9gj8LjWoACgUZWndQWz9y2efb+1mk1aj7XfXBvmrqoYqqk19zQnCi7gBAHxmWp/7fUCD daae6rZONeoujCRMDXVa63lmfLXiaVQHcQ6/wTueVefMjdYQKhxWlvXqUu1HQKwHo8jr Mv/wo0B+TKRbBQ+8yxkey8Yhnulrimi12Imkxd6eaiesZQT+pF6ZL1myiQCY4jYdbWua i6OQ== X-Gm-Message-State: AO0yUKU+ifZS5H7SHwdyZQS6f1QojGmEoIVYkWWZ3ATfeS/yPyNBhb+6 rPyAa5AyTWHS0iFgam0kEA0v9pEPA1Xs9F6jJDWKog== X-Google-Smtp-Source: AK7set94j2EgusYDIPFJpqlmZAE2h9JOS6PK3HKmO/GVyY/Ntoao0ZditMYj3Sh007jQx2hxudE5Fe41xltbBFtRcbA= X-Received: by 2002:a17:906:a04f:b0:8f1:4c6a:e77 with SMTP id bg15-20020a170906a04f00b008f14c6a0e77mr1886839ejb.12.1677234742696; Fri, 24 Feb 2023 02:32:22 -0800 (PST) MIME-Version: 1.0 References: <20230211064527.3481754-1-jstultz@google.com> <20230211064527.3481754-2-jstultz@google.com> <87o7porea9.ffs@tglx> <87a618qlcp.ffs@tglx> <87sff0ox1a.ffs@tglx> <874jrfq3jw.ffs@tglx> In-Reply-To: From: Michael Nazzareno Trimarchi Date: Fri, 24 Feb 2023 11:32:11 +0100 Message-ID: Subject: Re: [RFC][PATCH 2/2] time: alarmtimer: Use TASK_FREEZABLE to cleanup freezer handling To: Thomas Gleixner Cc: John Stultz , LKML , Stephen Boyd , Arnd Bergmann , Michael , kernel-team@android.com, Peter Zijlstra , "Rafael J. Wysocki" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi On Fri, Feb 24, 2023 at 11:02 AM Michael Nazzareno Trimarchi wrote: > > Hi Thomas > > On Tue, Feb 21, 2023 at 8:10 AM Michael Nazzareno Trimarchi > wrote: > > > > Hi > > > > On Tue, Feb 21, 2023 at 1:12 AM Thomas Gleixner wrote: > > > > > > Michael! > > > > > > On Mon, Feb 20 2023 at 22:32, Michael Nazzareno Trimarchi wrote: > > > > On Mon, Feb 20, 2023 at 10:18 PM Thomas Gleixner wrote: > > > >> * alarmtimer_fired - Handles alarm hrtimer being fired. > > > >> @@ -194,6 +196,8 @@ static enum hrtimer_restart alarmtimer_f > > > >> int ret = HRTIMER_NORESTART; > > > >> int restart = ALARMTIMER_NORESTART; > > > >> > > > >> + atomic_inc(&alarmtimer_wakeup); > > > >> + > > > > > > > > ptr->it_active = 0; > > > > if (ptr->it_interval) { > > > > atomic_inc(&alarmtimer_wakeup); > > > > si_private = ++ptr->it_requeue_pending; > > > > } > > > > > > > > Should I not go to the alarm_handle_timer? and only if it's a periodic > > > > one? > > > > > > Why? > > > > > > > You are right. I will pay more attention to my reply. > > > > I get time to test it and if the system suspend to ram we need to catch: > > case PM_SUSPEND_PREPARE: > case PM_POST_SUSPEND: > > Michael > > > Michael > > > > > Any alarmtimer which hits that window has exactly the same problem. > > > > > > It's not restricted to periodic timers. Why would a dropped one-shot > > > wakeup be acceptable? > > > > > > It's neither restricted to posix timers. If a clock_nanosleep(ALARM) > > > expires in that window then the task wake up will just end up in the > > > /dev/null bucket for the very same reason. Why would this be correct? > > > > > > Hmm? > > > > > > > > > > Michael > > > > > > > >> spin_lock_irqsave(&base->lock, flags); > > > [snip] I have something like this diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c index b68cb7f02a6b..b5f15e7f76cb 100644 --- a/kernel/time/alarmtimer.c +++ b/kernel/time/alarmtimer.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include "posix-timers.h" @@ -176,6 +177,7 @@ static void alarmtimer_dequeue(struct alarm_base *base, struct alarm *alarm) alarm->state &= ~ALARMTIMER_STATE_ENQUEUED; } +static atomic_t alarmtimer_wakeup; /** * alarmtimer_fired - Handles alarm hrtimer being fired. @@ -194,6 +196,8 @@ static enum hrtimer_restart alarmtimer_fired(struct hrtimer *timer) int ret = HRTIMER_NORESTART; int restart = ALARMTIMER_NORESTART; + atomic_inc(&alarmtimer_wakeup); + spin_lock_irqsave(&base->lock, flags); alarmtimer_dequeue(base, alarm); spin_unlock_irqrestore(&base->lock, flags); @@ -263,8 +267,18 @@ static int alarmtimer_suspend(struct device *dev) } } /* No timers to expire */ - if (min == KTIME_MAX) + if (min == KTIME_MAX) { + /* + * Handle wakeups which happened between the start of suspend and + * now as those wakeups might have tried to wake up a frozen task + * which means they are no longer in the alarm timer list. + */ + if (atomic_read(&alarmtimer_wakeup)) { + pm_wakeup_event(dev, 0); + return -EBUSY; + } return 0; + } if (ktime_to_ns(min) < 2 * NSEC_PER_SEC) { pm_wakeup_event(dev, 2 * MSEC_PER_SEC); @@ -296,6 +310,30 @@ static int alarmtimer_resume(struct device *dev) return 0; } +static int alarmtimer_pm_notifier_fn(struct notifier_block *bl, unsigned long state, + void *unused) +{ + switch (state) { + case PM_POST_SUSPEND: + atomic_set(&alarmtimer_wakeup, 0); + break; + } + return NOTIFY_DONE; +} + +static struct notifier_block alarmtimer_pm_notifier = { + .notifier_call = alarmtimer_pm_notifier_fn, +}; + +static inline int alarmtimer_register_pm_notifier(void) +{ + return register_pm_notifier(&alarmtimer_pm_notifier); +} + +static inline void alarmtimer_unregister_pm_notifier(void) +{ + unregister_pm_notifier(&alarmtimer_pm_notifier); +} #else static int alarmtimer_suspend(struct device *dev) { @@ -306,6 +344,15 @@ static int alarmtimer_resume(struct device *dev) { return 0; } + +static inline int alarmtimer_register_pm_notifier(void) +{ + return 0; +} + +static inline void alarmtimer_unregister_pm_notifier(void) +{ +} #endif static void @@ -904,11 +951,18 @@ static int __init alarmtimer_init(void) if (error) return error; - error = platform_driver_register(&alarmtimer_driver); + error = alarmtimer_register_pm_notifier(); if (error) goto out_if; + error = platform_driver_register(&alarmtimer_driver); + if (error) + goto out_pm; + return 0; + +out_pm: + alarmtimer_unregister_pm_notifier(); out_if: alarmtimer_rtc_interface_remove(); return error;