Re: [PATCH] xen/cpufreq: Reset policy after enabling/disabling turbo status

[Jane Malalane <Jane.Malalane@citrix.com>]

[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]


On 10/11/2021 11:48, Ian Jackson wrote:

[CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.

Jane Malalane writes ("[PATCH] xen/cpufreq: Reset policy after enabling/disabling turbo status"):


Before, user would change turbo status but this had no effect: boolean
was set but policy wasn't reevaluated.  Policy must be reevaluated so
that CPU frequency is chosen according to the turbo status and the
current governor.

Therefore, add __cpufreq_governor() in cpufreq_update_turbo().


...


Not taking this patch means that turbo status is misleading.

Taking this patch is low-risk as it only uses a function that already
exists and is already used for setting the chosen scaling governor.
Essentially, this change is equivalent to running 'xenpm
en/disable-turbo-mode' and, subsequently, running 'xenpm
set-scaling-governor <current governor>'.



Thanks.  I am positively inclined.  But I have one query about this
rationale.  Adding a new call to an existing function is OK if calling
__cpufreq_governor is permitted here.  Are there locking or reentrancy
hazards ?  Perhaps these issue have been considered but I would like
to see something explicit about that.

Thanks,


Hi Ian,


I think that's not a concern here because the only other
callers of __cpufreq_governor are __cpufreq_set_policy(), which is under the
same sysctl_lock, and cpufreq_del_cpu, which shouldn't be an issue because no
further action can be performed against the cpu when that function is called.


I will have to submit a v2 of this patch, so I can add
these considerations to the release rationale section?



Thanks,

Jane.



[-- Attachment #2: Type: text/html, Size: 2544 bytes --]