Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230711-cpuidle-v1-0-f391224b3140@gmail.com> <20230711-cpuidle-v1-2-f391224b3140@gmail.com>
In-Reply-To: <20230711-cpuidle-v1-2-f391224b3140@gmail.com>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Tue, 11 Jul 2023 19:55:46 +0200
Message-ID: <CAJZ5v0hBJoCHk_hFN2unez+Qi9v_zAFCZHXHa=Rb95pCFRLk6g@mail.gmail.com>
Subject: Re: [PATCH 2/2] PM: s2idle: Fully prevent the system from entering
 s2idle when cpuidle isn't supported
To:     Kazuki Hashimoto <kazukih0205@gmail.com>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
        Hector Martin <marcan@marcan.st>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Tue, Jul 11, 2023 at 7:54 AM Kazuki Hashimoto <kazukih0205@gmail.com> wrote:
>
> In order for systems to properly enter s2idle, we need functions both in
> the idle subsystem (such as call_cpuidle_s2idle()) and the suspend subsystem
> to be executed.
>
> s2idle got blocked in the idle subsystem on platforms without cpuidle after
> commit ef2b22ac540c ("cpuidle / sleep: Use broadcast timer for states that stop
> local timer").

What do you mean by "blocked in the idle subsystem"?

> However, the suspend subsystem doesn't have this, which can cause
> the suspend subsystem to begin entering s2idle behind the idle subsystem's back,

What do you mean by this?

> which in turn can cause the system to enter s2idle even though all the functions
> necessary for s2idle hasn't been executed, breaking the system
> (e.g. ClOCK_MONOTONIC keeps ticking during suspend even though it's not supposed
> to).

Why is this a problem?

> Prevent the system from entering s2idle when cpuidle isn't supported in the
> suspend subsystem as well.

I'm sure that there's a real problem you're trying to address, but I
cannot help you without understanding what the problem is.

So please explain what exactly is going on, what is expected to happen
and what happens instead and why this is problematic.

Till then, the patches are not going anywhere.

Thanks!