Re: [PATCH]cpuset: add new API to change cpuset top group's cpus

[Peter Zijlstra <peterz@infradead.org>]

On Tue, 2009-05-19 at 15:39 +0800, Shaohua Li wrote:
> ACPI 4.0 defines processor aggregator device. The device can notify OS to idle
> some CPUs to save power. This isn't to hot remove cpus, but just makes cpus
> idle.
> 
> This patch adds one API to change cpuset top group's cpus. If we want to
> make one cpu idle, simply remove the cpu from cpuset top group's cpu list,
> then all tasks will be migrate to other cpus, and other tasks will not be
> migrated to this cpu again. No functional changes.
> 
> We will use this API in new ACPI processor aggregator device driver later.

I don't think so. There really is a lot more to do than move processes
about.

Furthermore, I object to being able to remove online cpus from the top
cpuset, that just doesn't make sense.

I'd suggest using hotplug for this.

NAK

> Signed-off-by: Shaohua Li<shaohua.li@intel.com>
> ---
>  include/linux/cpuset.h |    5 +++++
>  kernel/cpuset.c        |   27 ++++++++++++++++++++++++---
>  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> Index: linux/kernel/cpuset.c
> ===================================================================
> --- linux.orig/kernel/cpuset.c	2009-05-12 16:27:16.000000000 +0800
> +++ linux/kernel/cpuset.c	2009-05-19 10:05:36.000000000 +0800
> @@ -929,14 +929,14 @@ static void update_tasks_cpumask(struct 
>   * @buf: buffer of cpu numbers written to this cpuset
>   */
>  static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
> -			  const char *buf)
> +			  const char *buf, bool top_ok)
>  {
>  	struct ptr_heap heap;
>  	int retval;
>  	int is_load_balanced;
>  
>  	/* top_cpuset.cpus_allowed tracks cpu_online_map; it's read-only */
> -	if (cs == &top_cpuset)
> +	if (cs == &top_cpuset && !top_ok)
>  		return -EACCES;
>  
>  	/*
> @@ -1496,7 +1496,7 @@ static int cpuset_write_resmask(struct c
>  
>  	switch (cft->private) {
>  	case FILE_CPULIST:
> -		retval = update_cpumask(cs, trialcs, buf);
> +		retval = update_cpumask(cs, trialcs, buf, false);
>  		break;
>  	case FILE_MEMLIST:
>  		retval = update_nodemask(cs, trialcs, buf);
> @@ -1511,6 +1511,27 @@ static int cpuset_write_resmask(struct c
>  	return retval;
>  }
>  
> +int cpuset_change_top_cpumask(const char *buf)
> +{
> +	int retval = 0;
> +	struct cpuset *cs = &top_cpuset;
> +	struct cpuset *trialcs;
> +
> +	if (!cgroup_lock_live_group(cs->css.cgroup))
> +		return -ENODEV;
> +
> +	trialcs = alloc_trial_cpuset(cs);
> +	if (!trialcs)
> +		return -ENOMEM;
> +
> +	retval = update_cpumask(cs, trialcs, buf, true);
> +
> +	free_trial_cpuset(trialcs);
> +	cgroup_unlock();
> +	return retval;
> +}
> +EXPORT_SYMBOL(cpuset_change_top_cpumask);
> +
>  /*
>   * These ascii lists should be read in a single call, by using a user
>   * buffer large enough to hold the entire map.  If read in smaller
> Index: linux/include/linux/cpuset.h
> ===================================================================
> --- linux.orig/include/linux/cpuset.h	2009-05-12 16:27:15.000000000 +0800
> +++ linux/include/linux/cpuset.h	2009-05-19 10:05:36.000000000 +0800
> @@ -92,6 +92,7 @@ extern void rebuild_sched_domains(void);
>  
>  extern void cpuset_print_task_mems_allowed(struct task_struct *p);
>  
> +extern int cpuset_change_top_cpumask(const char *buf);
>  #else /* !CONFIG_CPUSETS */
>  
>  static inline int cpuset_init_early(void) { return 0; }
> @@ -188,6 +189,10 @@ static inline void cpuset_print_task_mem
>  {
>  }
>  
> +static inline int cpuset_change_top_cpumask(const char *buf)
> +{
> +	return -ENODEV;
> +}
>  #endif /* !CONFIG_CPUSETS */
>  
>  #endif /* _LINUX_CPUSET_H */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/