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 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?

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.

Also, same question as to the amd folks; what serializes those static
variables?



[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