Re: [Xen-devel] [PATCH 05/10] x86/msr: Compile out unused logic/objects

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/02/2020 08:07, Jan Beulich wrote:
> On 26.02.2020 21:22, Andrew Cooper wrote:
>> @@ -76,16 +77,27 @@ void __init init_guest_msr_policy(void)
>>  {
>>      calculate_raw_policy();
>>      calculate_host_policy();
>> -    calculate_hvm_max_policy();
>> -    calculate_pv_max_policy();
>> +
>> +    if ( IS_ENABLED(CONFIG_PV) )
>> +        calculate_pv_max_policy();
>> +
>> +    if ( hvm_enabled )
>
> Any chance of talking you into doing things more symmetrically,
> by either also using IS_ENABLED(CONFIG_HVM) here or ...
>
>> +        calculate_hvm_max_policy();
>>  }
>>  
>>  int init_domain_msr_policy(struct domain *d)
>>  {
>> -    struct msr_policy *mp =
>> -        xmemdup(is_pv_domain(d) ?  &pv_max_msr_policy
>> -                                : &hvm_max_msr_policy);
>> +    struct msr_policy *mp = is_pv_domain(d)
>> +        ? (IS_ENABLED(CONFIG_PV)  ?  &pv_max_msr_policy : NULL)
>> +        : (IS_ENABLED(CONFIG_HVM) ? &hvm_max_msr_policy : NULL);
> ... (imo preferably) hvm_enabled here? Either way
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

The asymmetry is deliberate.

In the former hunk, hvm_enabled is short-circuited to false for
!CONFIG_HVM, and if I don't use hvm_enabled, here, then I've got to
retain the logic at the top of calculate_hvm_max_policy().  That seems
silly.

In this later hunk, we are looking for the most efficient way to allow
the compiler to discard the reference to hvm_max_msr_policy.  Using
hvm_enabled would be logically equivalent, but compile to more code in
CONFIG_HVM case, as it is a real boolean needing checking.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel