Re: [RESEND PATCH V9 3/7] cpufreq: amd-pstate: Enable amd-pstate preferred core supporting.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux