[PATCH 10/10] Xen: update pcpu online/offline logic

[PATCH 10/10] Xen: update pcpu online/offline logic

[Liu, Jinsong]

[-- Attachment #1: Type: text/plain, Size: 4987 bytes --]

>From 1e59d2ec2ff93b42c86d18c7cc9432ca2172a795 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Wed, 14 Dec 2011 21:06:11 +0800
Subject: [PATCH 10/10] Xen: update pcpu online/offline logic

This patch update pcpu online/offline logic.

It closes cpu0's /sys/.../xen_pcpu0/online authority to user.
It also simplifies pcpu sync by removing redundant logic.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
---
 drivers/xen/pcpu.c |   71 ++++++++++++++--------------------------------------
 1 files changed, 19 insertions(+), 52 deletions(-)

diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c
index 6d1a770..1d32784 100644
--- a/drivers/xen/pcpu.c
+++ b/drivers/xen/pcpu.c
@@ -24,7 +24,6 @@ static RAW_NOTIFIER_HEAD(xen_pcpu_chain);
 
 struct xen_pcpus {
 	struct list_head list;
-	int present;
 };
 static struct xen_pcpus xen_pcpus;
 
@@ -189,9 +188,13 @@ static int pcpu_sysdev_init(struct pcpu *cpu)
 		kfree(cpu);
 		return -1;
 	}
-	sysdev_create_file(&cpu->sysdev, &attr_online);
 	sysdev_create_file(&cpu->sysdev, &attr_apic_id);
 	sysdev_create_file(&cpu->sysdev, &attr_acpi_id);
+
+	/* Not open cpu0 online access to user */
+	if (cpu->sysdev.id)
+		sysdev_create_file(&cpu->sysdev, &attr_online);
+
 	return 0;
 }
 
@@ -239,17 +242,10 @@ static struct pcpu *init_pcpu(struct xenpf_pcpuinfo *info)
 	return pcpu;
 }
 
-#define PCPU_NO_CHANGE			0
-#define PCPU_ADDED			1
-#define PCPU_ONLINE_OFFLINE		2
-#define PCPU_REMOVED			3
 /*
  * Caller should hold the pcpu lock
- * < 0: Something wrong
- * 0: No changes
- * > 0: State changed
  */
-static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
+static int _sync_pcpu(int cpu_num, int *max_id)
 {
 	struct pcpu *pcpu = NULL;
 	struct xenpf_pcpuinfo *info;
@@ -259,14 +255,12 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
 	};
 	int ret;
 
-	*result = -1;
-
 	info = &op.u.pcpu_info;
 	info->xen_cpuid = cpu_num;
 
 	ret = HYPERVISOR_dom0_op(&op);
 	if (ret)
-		return NULL;
+		return -1;
 
 	if (max_id)
 		*max_id = op.u.pcpu_info.max_present;
@@ -275,28 +269,24 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
 
 	if (info->flags & XEN_PCPU_FLAGS_INVALID) {
 		/* The pcpu has been removed */
-		*result = PCPU_NO_CHANGE;
 		if (pcpu) {
 			raw_notifier_call_chain(&xen_pcpu_chain,
 			  XEN_PCPU_REMOVE,
 			  (void *)(long)pcpu->xen_id);
 			xen_pcpu_free(pcpu);
-			*result = PCPU_REMOVED;
 		}
-		return NULL;
+		return 0;
 	}
 
 
 	if (!pcpu) {
-		*result = PCPU_ADDED;
 		pcpu = init_pcpu(info);
 		if (pcpu == NULL) {
 			printk(KERN_WARNING "Failed to init pcpu %x\n",
 			  info->xen_cpuid);
-			  *result = -1;
+			return -1;
 		}
 	} else {
-		*result = PCPU_NO_CHANGE;
 		/*
 		 * Old PCPU is replaced with a new pcpu, this means
 		 * several virq is missed, will it happen?
@@ -307,10 +297,10 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
 			pcpu->apic_id = info->apic_id;
 			pcpu->acpi_id = info->acpi_id;
 		}
-		if (xen_pcpu_online_check(info, pcpu))
-			*result = PCPU_ONLINE_OFFLINE;
+		xen_pcpu_online_check(info, pcpu);
 	}
-	return pcpu;
+
+	return 0;
 }
 
 int xen_pcpu_index(uint32_t id, int is_acpiid)
@@ -353,50 +343,27 @@ static int xen_sync_pcpus(void)
 	/*
 	 * Boot cpu always have cpu_id 0 in xen
 	 */
-	int cpu_num = 0, max_id = 0, result = 0, present = 0;
+	int cpu_num = 0, max_id = 0, ret = 0;
 	struct list_head *elem, *tmp;
 	struct pcpu *pcpu;
 
 	get_pcpu_lock();
 
-	while ((result >= 0) && (cpu_num <= max_id)) {
-		pcpu = _sync_pcpu(cpu_num, &max_id, &result);
-
-		printk(KERN_DEBUG "sync cpu %x get result %x max_id %x\n",
-			cpu_num, result, max_id);
-
-		switch (result)	{
-		case PCPU_NO_CHANGE:
-			if (pcpu)
-				present++;
-			break;
-		case PCPU_ADDED:
-		case PCPU_ONLINE_OFFLINE:
-			present++;
-		case PCPU_REMOVED:
-			break;
-		default:
-			printk(KERN_WARNING "Failed to sync pcpu %x\n",
-			  cpu_num);
-			break;
-
-		}
+	while (!ret && (cpu_num <= max_id)) {
+		ret = _sync_pcpu(cpu_num, &max_id);
 		cpu_num++;
 	}
 
-	if (result < 0) {
+	if (ret < 0) {
 		list_for_each_safe(elem, tmp, &xen_pcpus.list) {
 			pcpu = list_entry(elem, struct pcpu, pcpu_list);
 			xen_pcpu_free(pcpu);
 		}
-		present = 0;
 	}
 
-	xen_pcpus.present = present;
-
 	put_pcpu_lock();
 
-	return 0;
+	return ret;
 }
 
 static void xen_pcpu_dpc(struct work_struct *work)
@@ -436,10 +403,10 @@ static int __init xen_pcpu_init(void)
 	}
 
 	INIT_LIST_HEAD(&xen_pcpus.list);
-	xen_pcpus.present = 0;
 
 	xen_sync_pcpus();
-	if (xen_pcpus.present > 0)
+
+	if (!list_empty(&xen_pcpus.list))
 		err = bind_virq_to_irqhandler(VIRQ_PCPU_STATE,
 			0, xen_pcpu_interrupt, 0, "pcpu", NULL);
 	if (err < 0)
-- 
1.6.5.6

[-- Attachment #2: 0010-Xen-update-pcpu-online-offline-logic.patch --]
[-- Type: application/octet-stream, Size: 4793 bytes --]

From 1e59d2ec2ff93b42c86d18c7cc9432ca2172a795 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Wed, 14 Dec 2011 21:06:11 +0800
Subject: [PATCH 10/10] Xen: update pcpu online/offline logic

This patch update pcpu online/offline logic.

It closes cpu0's /sys/.../xen_pcpu0/online authority to user.
It also simplifies pcpu sync by removing redundant logic.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
---
 drivers/xen/pcpu.c |   71 ++++++++++++++--------------------------------------
 1 files changed, 19 insertions(+), 52 deletions(-)

diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c
index 6d1a770..1d32784 100644
--- a/drivers/xen/pcpu.c
+++ b/drivers/xen/pcpu.c
@@ -24,7 +24,6 @@ static RAW_NOTIFIER_HEAD(xen_pcpu_chain);
 
 struct xen_pcpus {
 	struct list_head list;
-	int present;
 };
 static struct xen_pcpus xen_pcpus;
 
@@ -189,9 +188,13 @@ static int pcpu_sysdev_init(struct pcpu *cpu)
 		kfree(cpu);
 		return -1;
 	}
-	sysdev_create_file(&cpu->sysdev, &attr_online);
 	sysdev_create_file(&cpu->sysdev, &attr_apic_id);
 	sysdev_create_file(&cpu->sysdev, &attr_acpi_id);
+
+	/* Not open cpu0 online access to user */
+	if (cpu->sysdev.id)
+		sysdev_create_file(&cpu->sysdev, &attr_online);
+
 	return 0;
 }
 
@@ -239,17 +242,10 @@ static struct pcpu *init_pcpu(struct xenpf_pcpuinfo *info)
 	return pcpu;
 }
 
-#define PCPU_NO_CHANGE			0
-#define PCPU_ADDED			1
-#define PCPU_ONLINE_OFFLINE		2
-#define PCPU_REMOVED			3
 /*
  * Caller should hold the pcpu lock
- * < 0: Something wrong
- * 0: No changes
- * > 0: State changed
  */
-static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
+static int _sync_pcpu(int cpu_num, int *max_id)
 {
 	struct pcpu *pcpu = NULL;
 	struct xenpf_pcpuinfo *info;
@@ -259,14 +255,12 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
 	};
 	int ret;
 
-	*result = -1;
-
 	info = &op.u.pcpu_info;
 	info->xen_cpuid = cpu_num;
 
 	ret = HYPERVISOR_dom0_op(&op);
 	if (ret)
-		return NULL;
+		return -1;
 
 	if (max_id)
 		*max_id = op.u.pcpu_info.max_present;
@@ -275,28 +269,24 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
 
 	if (info->flags & XEN_PCPU_FLAGS_INVALID) {
 		/* The pcpu has been removed */
-		*result = PCPU_NO_CHANGE;
 		if (pcpu) {
 			raw_notifier_call_chain(&xen_pcpu_chain,
 			  XEN_PCPU_REMOVE,
 			  (void *)(long)pcpu->xen_id);
 			xen_pcpu_free(pcpu);
-			*result = PCPU_REMOVED;
 		}
-		return NULL;
+		return 0;
 	}
 
 
 	if (!pcpu) {
-		*result = PCPU_ADDED;
 		pcpu = init_pcpu(info);
 		if (pcpu == NULL) {
 			printk(KERN_WARNING "Failed to init pcpu %x\n",
 			  info->xen_cpuid);
-			  *result = -1;
+			return -1;
 		}
 	} else {
-		*result = PCPU_NO_CHANGE;
 		/*
 		 * Old PCPU is replaced with a new pcpu, this means
 		 * several virq is missed, will it happen?
@@ -307,10 +297,10 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
 			pcpu->apic_id = info->apic_id;
 			pcpu->acpi_id = info->acpi_id;
 		}
-		if (xen_pcpu_online_check(info, pcpu))
-			*result = PCPU_ONLINE_OFFLINE;
+		xen_pcpu_online_check(info, pcpu);
 	}
-	return pcpu;
+
+	return 0;
 }
 
 int xen_pcpu_index(uint32_t id, int is_acpiid)
@@ -353,50 +343,27 @@ static int xen_sync_pcpus(void)
 	/*
 	 * Boot cpu always have cpu_id 0 in xen
 	 */
-	int cpu_num = 0, max_id = 0, result = 0, present = 0;
+	int cpu_num = 0, max_id = 0, ret = 0;
 	struct list_head *elem, *tmp;
 	struct pcpu *pcpu;
 
 	get_pcpu_lock();
 
-	while ((result >= 0) && (cpu_num <= max_id)) {
-		pcpu = _sync_pcpu(cpu_num, &max_id, &result);
-
-		printk(KERN_DEBUG "sync cpu %x get result %x max_id %x\n",
-			cpu_num, result, max_id);
-
-		switch (result)	{
-		case PCPU_NO_CHANGE:
-			if (pcpu)
-				present++;
-			break;
-		case PCPU_ADDED:
-		case PCPU_ONLINE_OFFLINE:
-			present++;
-		case PCPU_REMOVED:
-			break;
-		default:
-			printk(KERN_WARNING "Failed to sync pcpu %x\n",
-			  cpu_num);
-			break;
-
-		}
+	while (!ret && (cpu_num <= max_id)) {
+		ret = _sync_pcpu(cpu_num, &max_id);
 		cpu_num++;
 	}
 
-	if (result < 0) {
+	if (ret < 0) {
 		list_for_each_safe(elem, tmp, &xen_pcpus.list) {
 			pcpu = list_entry(elem, struct pcpu, pcpu_list);
 			xen_pcpu_free(pcpu);
 		}
-		present = 0;
 	}
 
-	xen_pcpus.present = present;
-
 	put_pcpu_lock();
 
-	return 0;
+	return ret;
 }
 
 static void xen_pcpu_dpc(struct work_struct *work)
@@ -436,10 +403,10 @@ static int __init xen_pcpu_init(void)
 	}
 
 	INIT_LIST_HEAD(&xen_pcpus.list);
-	xen_pcpus.present = 0;
 
 	xen_sync_pcpus();
-	if (xen_pcpus.present > 0)
+
+	if (!list_empty(&xen_pcpus.list))
 		err = bind_virq_to_irqhandler(VIRQ_PCPU_STATE,
 			0, xen_pcpu_interrupt, 0, "pcpu", NULL);
 	if (err < 0)
-- 
1.6.5.6

Re: [Xen-devel] [PATCH 10/10] Xen: update pcpu online/offline logic

[Konrad Rzeszutek Wilk]

On Fri, Dec 23, 2011 at 10:32:12AM +0000, Liu, Jinsong wrote:
> >From 1e59d2ec2ff93b42c86d18c7cc9432ca2172a795 Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Wed, 14 Dec 2011 21:06:11 +0800
> Subject: [PATCH 10/10] Xen: update pcpu online/offline logic
> 
> This patch update pcpu online/offline logic.

Roll them in the patch you posted please.
> 
> It closes cpu0's /sys/.../xen_pcpu0/online authority to user.
> It also simplifies pcpu sync by removing redundant logic.
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
> ---
>  drivers/xen/pcpu.c |   71 ++++++++++++++--------------------------------------
>  1 files changed, 19 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c
> index 6d1a770..1d32784 100644
> --- a/drivers/xen/pcpu.c
> +++ b/drivers/xen/pcpu.c
> @@ -24,7 +24,6 @@ static RAW_NOTIFIER_HEAD(xen_pcpu_chain);
>  
>  struct xen_pcpus {
>  	struct list_head list;
> -	int present;
>  };
>  static struct xen_pcpus xen_pcpus;
>  
> @@ -189,9 +188,13 @@ static int pcpu_sysdev_init(struct pcpu *cpu)
>  		kfree(cpu);
>  		return -1;
>  	}
> -	sysdev_create_file(&cpu->sysdev, &attr_online);
>  	sysdev_create_file(&cpu->sysdev, &attr_apic_id);
>  	sysdev_create_file(&cpu->sysdev, &attr_acpi_id);
> +
> +	/* Not open cpu0 online access to user */
> +	if (cpu->sysdev.id)
> +		sysdev_create_file(&cpu->sysdev, &attr_online);
> +
>  	return 0;
>  }
>  
> @@ -239,17 +242,10 @@ static struct pcpu *init_pcpu(struct xenpf_pcpuinfo *info)
>  	return pcpu;
>  }
>  
> -#define PCPU_NO_CHANGE			0
> -#define PCPU_ADDED			1
> -#define PCPU_ONLINE_OFFLINE		2
> -#define PCPU_REMOVED			3
>  /*
>   * Caller should hold the pcpu lock
> - * < 0: Something wrong
> - * 0: No changes
> - * > 0: State changed
>   */
> -static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
> +static int _sync_pcpu(int cpu_num, int *max_id)
>  {
>  	struct pcpu *pcpu = NULL;
>  	struct xenpf_pcpuinfo *info;
> @@ -259,14 +255,12 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
>  	};
>  	int ret;
>  
> -	*result = -1;
> -
>  	info = &op.u.pcpu_info;
>  	info->xen_cpuid = cpu_num;
>  
>  	ret = HYPERVISOR_dom0_op(&op);
>  	if (ret)
> -		return NULL;
> +		return -1;
>  
>  	if (max_id)
>  		*max_id = op.u.pcpu_info.max_present;
> @@ -275,28 +269,24 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
>  
>  	if (info->flags & XEN_PCPU_FLAGS_INVALID) {
>  		/* The pcpu has been removed */
> -		*result = PCPU_NO_CHANGE;
>  		if (pcpu) {
>  			raw_notifier_call_chain(&xen_pcpu_chain,
>  			  XEN_PCPU_REMOVE,
>  			  (void *)(long)pcpu->xen_id);
>  			xen_pcpu_free(pcpu);
> -			*result = PCPU_REMOVED;
>  		}
> -		return NULL;
> +		return 0;
>  	}
>  
>  
>  	if (!pcpu) {
> -		*result = PCPU_ADDED;
>  		pcpu = init_pcpu(info);
>  		if (pcpu == NULL) {
>  			printk(KERN_WARNING "Failed to init pcpu %x\n",
>  			  info->xen_cpuid);
> -			  *result = -1;
> +			return -1;
>  		}
>  	} else {
> -		*result = PCPU_NO_CHANGE;
>  		/*
>  		 * Old PCPU is replaced with a new pcpu, this means
>  		 * several virq is missed, will it happen?
> @@ -307,10 +297,10 @@ static struct pcpu *_sync_pcpu(int cpu_num, int *max_id, int *result)
>  			pcpu->apic_id = info->apic_id;
>  			pcpu->acpi_id = info->acpi_id;
>  		}
> -		if (xen_pcpu_online_check(info, pcpu))
> -			*result = PCPU_ONLINE_OFFLINE;
> +		xen_pcpu_online_check(info, pcpu);
>  	}
> -	return pcpu;
> +
> +	return 0;
>  }
>  
>  int xen_pcpu_index(uint32_t id, int is_acpiid)
> @@ -353,50 +343,27 @@ static int xen_sync_pcpus(void)
>  	/*
>  	 * Boot cpu always have cpu_id 0 in xen
>  	 */
> -	int cpu_num = 0, max_id = 0, result = 0, present = 0;
> +	int cpu_num = 0, max_id = 0, ret = 0;
>  	struct list_head *elem, *tmp;
>  	struct pcpu *pcpu;
>  
>  	get_pcpu_lock();
>  
> -	while ((result >= 0) && (cpu_num <= max_id)) {
> -		pcpu = _sync_pcpu(cpu_num, &max_id, &result);
> -
> -		printk(KERN_DEBUG "sync cpu %x get result %x max_id %x\n",
> -			cpu_num, result, max_id);
> -
> -		switch (result)	{
> -		case PCPU_NO_CHANGE:
> -			if (pcpu)
> -				present++;
> -			break;
> -		case PCPU_ADDED:
> -		case PCPU_ONLINE_OFFLINE:
> -			present++;
> -		case PCPU_REMOVED:
> -			break;
> -		default:
> -			printk(KERN_WARNING "Failed to sync pcpu %x\n",
> -			  cpu_num);
> -			break;
> -
> -		}
> +	while (!ret && (cpu_num <= max_id)) {
> +		ret = _sync_pcpu(cpu_num, &max_id);
>  		cpu_num++;
>  	}
>  
> -	if (result < 0) {
> +	if (ret < 0) {
>  		list_for_each_safe(elem, tmp, &xen_pcpus.list) {
>  			pcpu = list_entry(elem, struct pcpu, pcpu_list);
>  			xen_pcpu_free(pcpu);
>  		}
> -		present = 0;
>  	}
>  
> -	xen_pcpus.present = present;
> -
>  	put_pcpu_lock();
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static void xen_pcpu_dpc(struct work_struct *work)
> @@ -436,10 +403,10 @@ static int __init xen_pcpu_init(void)
>  	}
>  
>  	INIT_LIST_HEAD(&xen_pcpus.list);
> -	xen_pcpus.present = 0;
>  
>  	xen_sync_pcpus();
> -	if (xen_pcpus.present > 0)
> +
> +	if (!list_empty(&xen_pcpus.list))
>  		err = bind_virq_to_irqhandler(VIRQ_PCPU_STATE,
>  			0, xen_pcpu_interrupt, 0, "pcpu", NULL);
>  	if (err < 0)
> -- 
> 1.6.5.6


> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel