On 11.10.2022 18:22, Roger Pau Monné wrote:
> On Thu, Aug 18, 2022 at 03:03:33PM +0200, Jan Beulich wrote:
>> @@ -1309,6 +1360,39 @@ static int __init mwait_idle_probe(void)
>> pr_debug(PREFIX "lapic_timer_reliable_states %#x\n",
>> lapic_timer_reliable_states);
>>
>> + str = preferred_states;
>> + if (isdigit(str[0]))
>> + preferred_states_mask = simple_strtoul(str, &str, 0);
>> + else if (str[0])
>> + {
>> + const char *ss;
>> +
>> + do {
>> + const struct cpuidle_state *state = icpu->state_table;
>> + unsigned int bit = 1;
>> +
>> + ss = strchr(str, ',');
>> + if (!ss)
>> + ss = strchr(str, '\0');
>> +
>> + for (; state->name[0]; ++state) {
>> + bit <<= 1;
>> + if (!cmdline_strcmp(str, state->name)) {
>> + preferred_states_mask |= bit;
>> + break;
>> + }
>> + }
>> + if (!state->name[0])
>> + break;
>> +
>> + str = ss + 1;
>> + } while (*ss);
>> +
>> + str -= str == ss + 1;
>
> I would add parentheses to the sum for clarity.
If I was to add parentheses here, then like this:
str -= (str == ss + 1);
Looks like I've screwed up with indentation here, though.
>> @@ -1400,8 +1484,18 @@ static int cf_check mwait_idle_cpu_init(
>> if (icpu->byt_auto_demotion_disable_flag)
>> on_selected_cpus(cpumask_of(cpu), byt_auto_demotion_disable,
>> NULL, 1);
>>
>> - if (icpu->disable_promotion_to_c1e)
>> + switch (icpu->c1e_promotion) {
>> + case C1E_PROMOTION_DISABLE:
>> on_selected_cpus(cpumask_of(cpu), c1e_promotion_disable, NULL,
>> 1);
>> + break;
>> +
>> + case C1E_PROMOTION_ENABLE:
>> + on_selected_cpus(cpumask_of(cpu), c1e_promotion_enable, NULL,
>> 1);
>> + break;
>> +
>> + case C1E_PROMOTION_PRESERVE:
>> + break;
>> + }
>
> I find it kind of weird to user a notifier for this, won't it be
> easier to set the C1E promotion as part of the CPU bringup process?
A CPU notifier _is_ part of the CPU bringup process, isn't it? So it's
not clear to me what alternative you're thinking of.
> I see we also set other bits in the same way.
Well, yes - right here I only extend what we already have in place.
Re-working that in whichever way ought to be a separate topic imo, and
preferably not be part of a port of a commit coming from Linux.
Jan
