| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20090901214935.bc505f9f.akpm@linux-foundation.org>
Date: Tue, 1 Sep 2009 21:49:35 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Gautham R Shenoy <ego@...ibm.com>
Cc: Joel Schopp <jschopp@...tin.ibm.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Vaidyanathan Srinivasan <svaidy@...ux.vnet.ibm.com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Dipankar Sarma <dipankar@...ibm.com>,
Balbir Singh <balbir@...ibm.com>,
Venkatesh Pallipadi <venkatesh.pallipadi@...el.com>,
linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
"Darrick J. Wong" <djwong@...ibm.com>
Subject: Re: [PATCH v2 1/2] cpu: Offline state Framework.
On Fri, 28 Aug 2009 15:30:16 +0530 Gautham R Shenoy <ego@...ibm.com> wrote:
> Provide an interface by which the system administrator can decide what state
> should the CPU go to when it is offlined.
>
> To query the hotplug states, on needs to perform a read on the sysfs tunable:
> /sys/devices/system/cpu/cpu<number>/available_hotplug_states
>
> To query or set the current state for a particular CPU, one needs to
> use the sysfs interface:
> /sys/devices/system/cpu/cpu<number>/current_state
>
> This patch implements the architecture independent bits of the
> cpu-offline-state framework.
>
> Architectures which want to expose the multiple offline-states to the
> userspace are expected to write a driver which can register
> with this framework.
>
> Such a driver should:
> - Implement the callbacks defined in the structure struct cpu_offline_driver
> which can be called into by this framework when the corresponding
> sysfs interfaces are read or written into.
>
> - Ensure that the following operation puts the CPU in the same state
> as it did in the absence of the driver.
> echo 0 > /sys/devices/system/cpu/cpu<number>/online
>
> This framework also serializes the writes to the "current_state"
> with respect to with the writes to the "online" sysfs tunable.
>
It would be nice to document this new userspace interface somewhere.
> +struct cpu_offline_driver *cpu_offline_driver;
> +static DEFINE_MUTEX(cpu_offline_driver_lock);
> +
> +ssize_t show_available_states(struct sys_device *dev,
> + struct sysdev_attribute *attr, char *buf)
> +{
> + struct cpu *cpu = container_of(dev, struct cpu, sysdev);
> + int cpu_num = cpu->sysdev.id;
> + ssize_t ret;
> +
> + mutex_lock(&cpu_offline_driver_lock);
> + if (!cpu_offline_driver) {
> + ret = -EEXIST;
> + goto out_unlock;
> + }
> +
> + ret = cpu_offline_driver->read_available_states(cpu_num, buf);
> +
> +out_unlock:
> + mutex_unlock(&cpu_offline_driver_lock);
> +
> + return ret;
> +
> +}
The patch adds boatloads of global symbols which do not have names
which are appropriate for global symbols.
> +ssize_t show_current_state(struct sys_device *dev,
> + struct sysdev_attribute *attr, char *buf)
Like that.
> +ssize_t store_current_state(struct sys_device *dev,
> + struct sysdev_attribute *attr,
> + const char *buf, size_t count)
And that.
> +
> +static SYSDEV_ATTR(available_hotplug_states, 0444, show_available_states,
> + NULL);
> +static SYSDEV_ATTR(current_state, 0644, show_current_state,
> + store_current_state);
> +
> +/* Should be called with cpu_offline_driver_lock held */
> +void cpu_offline_driver_add_cpu(struct sys_device *cpu_sys_dev)
> +{
> + if (!cpu_offline_driver || !cpu_sys_dev)
> + return;
> +
> + sysdev_create_file(cpu_sys_dev, &attr_available_hotplug_states);
> + sysdev_create_file(cpu_sys_dev, &attr_current_state);
> +}
> +
> +/* Should be called with cpu_offline_driver_lock held */
> +void cpu_offline_driver_remove_cpu(struct sys_device *cpu_sys_dev)
> +{
> + if (!cpu_offline_driver || !cpu_sys_dev)
> + return;
> +
> + sysdev_remove_file(cpu_sys_dev, &attr_available_hotplug_states);
> + sysdev_remove_file(cpu_sys_dev, &attr_current_state);
> +
> +}
Please don't just ignore possible error returns.
> +int register_cpu_offline_driver(struct cpu_offline_driver *arch_cpu_driver)
> +{
> + int ret = 0;
> + int cpu;
> + mutex_lock(&cpu_offline_driver_lock);
> +
The blank line goes after end-of-locals and before start-of-code.
> + if (cpu_offline_driver != NULL) {
> + ret = -EEXIST;
> + goto out_unlock;
> + }
> +
> + if (!(arch_cpu_driver->read_available_states &&
> + arch_cpu_driver->read_current_state &&
> + arch_cpu_driver->write_current_state)) {
> + ret = -EINVAL;
> + goto out_unlock;
This seems pretty pointless. Just let the code oops - the developer
will notice fairly quickly.
> + }
> +
> + cpu_offline_driver = arch_cpu_driver;
> +
> + for_each_possible_cpu(cpu)
> + cpu_offline_driver_add_cpu(get_cpu_sysdev(cpu));
> +
> +out_unlock:
> + mutex_unlock(&cpu_offline_driver_lock);
> + return ret;
> +}
> +
> +void unregister_cpu_offline_driver(struct cpu_offline_driver *arch_cpu_driver)
> +{
> + int cpu;
> + mutex_lock(&cpu_offline_driver_lock);
> +
> + if (!cpu_offline_driver) {
> + WARN_ON(1);
if (WARN_ON(!cpu_offline_driver)) {
> + mutex_unlock(&cpu_offline_driver_lock);
> + return;
> + }
> +
> + for_each_possible_cpu(cpu)
> + cpu_offline_driver_remove_cpu(get_cpu_sysdev(cpu));
> +
> + cpu_offline_driver = NULL;
> + mutex_unlock(&cpu_offline_driver_lock);
> +}
> +
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists