On 10/16/2023 12:58 PM, Peter Zijlstra wrote:
On Mon, Oct 16, 2023 at 06:20:53AM +0000, Meng, Li (Jassmine) wrote:
+static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata) {
+ int ret, prio;
+ u32 highest_perf;
+ static u32 max_highest_perf = 0, min_highest_perf = U32_MAX;
What serializes these things?
Also, *why* are you using u32 here, what's wrong with something like:
int max_hp = INT_MIN, min_hp = INT_MAX;
[Meng, Li (Jassmine)]
We use ITMT architecture to utilize preferred core features.
Therefore, we need to try to be consistent with Intel's implementation
as much as possible. For details, please refer to the
intel_pstate_set_itmt_prio function in file intel_pstate.c. (Line 355)
I think using the data type of u32 is consistent with the data
structures of cppc_perf_ctrls and amd_cpudata etc.
Rafael, should we fix intel_pstate too?
Srinivas should be more familiar with this code than I am, so adding him.
The point is, that sched_asym_prefer(), the final consumer of these
values uses int and thus an explicitly signed compare.
Using u32 and U32_MAX anywhere near the setting the priority makes
absolutely no sense.
If you were to have the high bit set, things do not behave as expected.
Right, but in practice these values are always between 0 and 255
inclusive AFAICS.
It would have been better to use u8 I suppose.
Also, same question as to the amd folks; what serializes those static
variables?
That's a good one.
