Messages in this thread | | | Date | Thu, 12 Jan 2023 12:31:01 +0100 | Subject | Re: [PATCH v2 1/5] PM: domains: Add GENPD_FLAG_RT_SAFE for PREEMPT_RT | From | Krzysztof Kozlowski <> |
| |
On 12/01/2023 11:32, Sebastian Andrzej Siewior wrote: > On 2022-12-19 16:14:59 [+0100], Krzysztof Kozlowski wrote: >> Realtime kernels with PREEMPT_RT must use raw_spinlock_t for domains >> which are invoked from CPU idle (thus from atomic section). Example is >> cpuidle PSCI domain driver which itself is PREEMPT_RT safe, but is being >> called as part of cpuidle. > > I think it needs to be clarified what PREEMPT_RT safe means.
OK
> PSCI is an > external interface which does not inform us what it does and how long > the operation will take. > The ACPI table for instance populate several idle states and their > entry/exit time. Then you can decide if and when an entry/exit latency > of 500us is PREEMPT_RT safe. > >> Add a flag allowing a power domain provider to indicate it is RT safe. >> The flag is supposed to be used with existing GENPD_FLAG_IRQ_SAFE. >> >> Cc: Adrien Thierry <athierry@redhat.com> >> Cc: Brian Masney <bmasney@redhat.com> >> Cc: linux-rt-users@vger.kernel.org >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> > … >> index 1cd41bdf73cf..0a1600244963 100644 >> --- a/include/linux/pm_domain.h >> +++ b/include/linux/pm_domain.h >> @@ -61,6 +61,14 @@ >> * GENPD_FLAG_MIN_RESIDENCY: Enable the genpd governor to consider its >> * components' next wakeup when determining the >> * optimal idle state. >> + * >> + * GENPD_FLAG_RT_SAFE: When used with GENPD_FLAG_IRQ_SAFE, this informs >> + * genpd that its backend callbacks, ->power_on|off(), >> + * do not use other spinlocks. They might use >> + * raw_spinlocks or other pre-emption-disable >> + * methods, all of which are PREEMPT_RT safe. Note > > Please use spinlock_t and raw_spinlock_t. Wouldn't it be better to write > "preemption" instead "pre-emption"?
Sure.
> The important part is probably that once a raw_spinlock_t has been > acquired, it is not possible to invoke any function that acquries > sleeping locks (which includes memory allocations). While even without > that flag it is possible to invoke a function which disables and enables > preemption on its own. > >> + * that, a genpd having this flag set, requires its >> + * masterdomains to also have it set. > > This could be verified upon registration, no?
It is, just like the IRQ_SAFE flag. The code is symmetrical to IRQ_SAFE.
> It might be worth noting that preemption-off section during PM > operations contribute to the system's max latency.
You mean in the commit msg? In the doc, I don't want to deviate from IRQ_SAFE. It's not really related to the flag.
> Depending on how low > the operation is, it may or may not be a problem. > The ->power_on|off() refers to the sate of the component, right?
It refers to genpd framework.
Best regards, Krzysztof
| |