Messages in this thread | | | Date | Wed, 29 Mar 2017 08:24:59 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH V4 2/2] perf/x86: add sysfs entry to freeze counter on SMI |
| |
* kan.liang@intel.com <kan.liang@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
| |