Re: [Powerpc-utils-devel] [PATCH] drmgr: Fix CPU/LMB add/removal using drc_index

[Nathan F. <nf...@li...>]

Applied.

Thanks,
-Nathan

On 04/06/2015 11:01 PM, Bharata B Rao wrote:
> From: Bharata B Rao <bh...@li...>
> 
> This patch fixes a few things:
> 
> - Removing a CPU using drc_index is currently removing wrong CPU since
>   CPU removal code doesn't lookup CPU by index. Introduce a routine to
>   lookup by index and ensure that it gets used from the removal path.
> - get_cpu_by_name() does an implicit looking up of index also. Fix this.
> - drmgr -s option can mean both drc_index and drc_name. In case of
>   drc_index, rtas_errd will invoke drmgr without -q option which results
>   in no CPU or LMB being added. Make 1 as the default count of
>   CPUs unconditionally which applies to both the cases.
> 
> After these fixes, I am able to get CPU hotplug and removal working
> with QEMU on a PowerKVM host.
> 
> Signed-off-by: Bharata B Rao <bh...@li...>
> ---
>  src/drmgr/drslot_chrp_cpu.c | 44 ++++++++++++++++++++++++++++++++++++--------
>  src/drmgr/drslot_chrp_mem.c |  2 +-
>  2 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/src/drmgr/drslot_chrp_cpu.c b/src/drmgr/drslot_chrp_cpu.c
> index e6f185d..54fc576 100644
> --- a/src/drmgr/drslot_chrp_cpu.c
> +++ b/src/drmgr/drslot_chrp_cpu.c
> @@ -44,16 +44,25 @@ static struct dr_node *
>  get_cpu_by_name(struct dr_info *drinfo, const char *name)
>  {
>  	struct dr_node *cpu;
> -	uint32_t drc_index;
> 
>  	for (cpu = drinfo->all_cpus; cpu; cpu = cpu->next) {
> -		if (strcmp(cpu->drc_name, name) == 0)
> +		if (strcmp(cpu->drc_name, name) == 0) {
>  			break;
> +		}
> +	}
> 
> -		/* See if the drc index was specified */
> -		drc_index = strtoul(name, NULL, 0);
> -		if (cpu->drc_index == drc_index)
> +	return cpu;
> +}
> +
> +static struct dr_node *
> +get_cpu_by_index(struct dr_info *drinfo, uint32_t index)
> +{
> +	struct dr_node *cpu;
> +
> +	for (cpu = drinfo->all_cpus; cpu; cpu = cpu->next) {
> +		if (cpu->drc_index == index) {
>  			break;
> +		}
>  	}
> 
>  	return cpu;
> @@ -105,6 +114,13 @@ get_available_cpu(struct options *opts, struct dr_info *dr_info)
>  			    opts->usr_drc_name);
> 
>  		return cpu;
> +	} else if (opts->usr_drc_index) {
> +		cpu = get_cpu_by_index(dr_info, opts->usr_drc_index);
> +		if (!cpu)
> +			say(ERROR, "Could not locate cpu %x\n",
> +			    opts->usr_drc_index);
> +
> +		return cpu;
>  	}
> 
>  	switch (opts->action) {
> @@ -261,6 +277,7 @@ static int
>  smt_threads_func(struct options *opts, struct dr_info *dr_info)
>  {
>  	int rc;
> +	struct dr_node *cpu;
> 
>  	if (opts->quantity != 1) {
>  		say(ERROR, "Quantity option '-q' may not be specified with "
> @@ -274,8 +291,6 @@ smt_threads_func(struct options *opts, struct dr_info *dr_info)
>  	}
> 
>  	if (opts->usr_drc_name) {
> -		struct dr_node *cpu;
> -
>  		cpu = get_cpu_by_name(dr_info, opts->usr_drc_name);
>  		if (cpu == NULL) {
>  			say(ERROR, "Could not find cpu %s\n",
> @@ -288,6 +303,19 @@ smt_threads_func(struct options *opts, struct dr_info *dr_info)
>  		else if (opts->action == REMOVE)
>  			rc = cpu_disable_smt(cpu);
> 
> +	} else if (opts->usr_drc_index) {
> +		cpu = get_cpu_by_index(dr_info, opts->usr_drc_index);
> +		if (cpu == NULL) {
> +			say(ERROR, "Could not find cpu %x\n",
> +			    opts->usr_drc_index);
> +			return -1;
> +		}
> +
> +		if (opts->action == ADD)
> +			rc = cpu_enable_smt(cpu, dr_info);
> +		else if (opts->action == REMOVE)
> +			rc = cpu_disable_smt(cpu);
> +
>  	} else { /* no drc name given, action is system-wide */
>  		if (opts->action == ADD)
>  			rc = system_enable_smt(dr_info);
> @@ -302,7 +330,7 @@ int
>  valid_cpu_options(struct options *opts)
>  {
>  	/* default to a quantity of 1 */
> -	if ((opts->quantity == 0) && (opts->usr_drc_name == NULL))
> +	if ((opts->quantity == 0))
>  		opts->quantity = 1;
> 
>  	if ((opts->action != ADD) && (opts->action != REMOVE)) {
> diff --git a/src/drmgr/drslot_chrp_mem.c b/src/drmgr/drslot_chrp_mem.c
> index d6ece97..136219e 100644
> --- a/src/drmgr/drslot_chrp_mem.c
> +++ b/src/drmgr/drslot_chrp_mem.c
> @@ -1211,7 +1211,7 @@ int
>  valid_mem_options(struct options *opts)
>  {
>  	/* default to a quantity of 1 */
> -	if ((opts->quantity == 0) && (opts->usr_drc_name == NULL))
> +	if (opts->quantity == 0)
>  		opts->quantity = 1;
> 
>  	if ((opts->action != ADD) && (opts->action != REMOVE)) {
>