lkml.org 
[lkml]   [2017]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2017-03-29 08:26    [W:0.079 / U:1.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site