Re: [PATCH V4 2/2] perf/x86: add sysfs entry to freeze counter on SMI

[Ingo Molnar]

* kan.li...@intel.com <kan.li...@intel.com> wrote:

> +static void flip_smm_bit(void *data)
> +{
> +     int val = *(int *)data;
> +
> +     msr_flip_bit(MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT, 
> (bool)val);
> +}

BTW., you can probably shorten that and remove a type cast by using a more 
natural 
type for 'val':

static void flip_smm_bit(void *data)
{
        bool set = *(int *)data;

        msr_flip_bit(MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT, 
set);
}

Also note that 'set' is the more natural local variable name here as well, as 
it 
matches the parameter name of the msr_flip_bit() function.

BTW. #2, "MSR_IA32_DEBUGCTLMSR" is a bit of a misnomer, why is 'MSR' mentioned 
twice? If it was 'MSR_IA32_DEBUGCTL' then it could all be:

        msr_flip_bit(MSR_IA32_DEBUGCTL, DEBUGCTL_FREEZE_WHILE_SMM_BIT, set);

plus if 'FREEZE_WHILE_SMM' is renamed to 'FREEZE_IN_SMM', we'd have:

        msr_flip_bit(MSR_IA32_DEBUGCTL, DEBUGCTL_FREEZE_IN_SMM_BIT, set);

... which, incidentally, fits into 80 cols nicely.

But that's unrelated to your patch, you just made canonical use of the existing 
nomenclature.

Thanks,

        Ingo