lkml.org 
[lkml]   [2019]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 4/5] x86/umwait: Add sysfs interface to control umwait maximum time
On Wed, 19 Jun 2019, Fenghua Yu wrote:

> Users can write an unsigned 32-bit number to
> /sys/devices/system/cpu/umwait_control/max_time to change the default

Users? Administrators can. Users NOT.

> value. Note that a value of zero means there is no limit. Low order
> two bits must be zero.

...

> +static ssize_t max_time_store(struct device *kobj,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u32 max_time;
> + int ret;
> +
> + ret = kstrtou32(buf, 0, &max_time);
> + if (ret)
> + return ret;
> +
> + /* bits[1:0] must be zero */
> + if (max_time & ~MSR_IA32_UMWAIT_CONTROL_MAX_TIME)
> + return -EINVAL;
> +
> + mutex_lock(&umwait_lock);
> +
> + if (max_time == get_umwait_ctrl_max_time())
> + goto out_unlock;
> +
> + WRITE_ONCE(umwait_control_cached,
> + UMWAIT_CTRL_VAL(max_time, get_umwait_ctrl_c02()));

Same convoluted logic with reading the cached value twice to confuse the
reader.

ctrl = READ_ONCE(umwait_control_cached);
if (max_time == umwait_ctrl_max_time(ctrl))
goto out_unlock;

ctrl = (ctrl & ~MSR_IA32_UMWAIT_CONTROL_TIME_MASK) | max_time;
WRITE_ONCE(umwait_control_cached, ctrl);

Simple, right?

But this can be done even simpler with a shared update function:

static void umwait_update_control(u32 maxtime, bool c02_enable)
{
u32 ctrl = maxtime & MSR_IA32_UMWAIT_CONTROL_TIME_MASK;

if (!c02_enable)
ctrl |= MSR_IA32_UMWAIT_CONTROL_C02_DISABLE;

WRITE_ONCE(umwait_control_cached, ctrl);
/* Propagate to all CPUs */
on_each_cpu(umwait_update_control_msr, NULL, 1);
}

With that both functions become trivial and do not have duplicated code.

Thanks,

tglx

\
 
 \ /
  Last update: 2019-06-24 05:03    [W:1.124 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site