Re: [PATCH v4 3/5] x86/umwait: Add sysfs interface to control umwait C0.2 state

[Fenghua Yu]

On Sat, Jun 08, 2019 at 03:52:03PM -0700, Andy Lutomirski wrote:
> On Fri, Jun 7, 2019 at 3:10 PM Fenghua Yu <fenghua.yu@xxxxxxxxx> wrote:
> >
> > C0.2 state in umwait and tpause instructions can be enabled or disabled
> > on a processor through IA32_UMWAIT_CONTROL MSR register.
> >
> 
> > +static u32 get_umwait_control_c02(void)
> > +{
> > +       return umwait_control_cached & MSR_IA32_UMWAIT_CONTROL_C02;
> > +}
> > +
> > +static u32 get_umwait_control_max_time(void)
> > +{
> > +       return umwait_control_cached & MSR_IA32_UMWAIT_CONTROL_MAX_TIME;
> > +}
> > +
> 
> I'm not convinced that these helpers make the code any more readable.

The helpers reduce length of statements that call them. Otherwise, all of
the statements would be easily over 80 characters.

Plus, each of the helpers is called multiple places in #0003 and #0004.
So the helpers make the patches smaller and cleaner.

So is it still OK to keep the helpers?

Thanks.

-Fenghua