[PATCH v2 1/5] PM: domains: Add GENPD_FLAG_RT_SAFE for PREEMPT_RT

[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 at redhat.com>
>> Cc: Brian Masney <bmasney at redhat.com>
>> Cc: linux-rt-users at vger.kernel.org
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski at 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