From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: RE: [PATCH][DOM0] Expose physical CPU information
	in dom0
Date: Thu, 12 Nov 2009 10:46:58 -0500
Message-ID: <20091112154658.GA32409@phenom.dumpdata.com>
References: <E2263E4A5B2284449EEBD0AAB751098418E4775B9B@PDSMSX501.ccr.corp.intel.com>
	<E2263E4A5B2284449EEBD0AAB751098418E55575DA@PDSMSX501.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Return-path: <xen-devel-bounces@lists.xensource.com>
Content-Disposition: inline
In-Reply-To: <E2263E4A5B2284449EEBD0AAB751098418E55575DA@PDSMSX501.ccr.corp.intel.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: "Jiang, Yunhong" <yunhong.jiang@intel.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
List-Id: xen-devel@lists.xenproject.org


Some little comments..

.. snip ..

> +	if (info->flags & XEN_PCPU_FLAGS_INVALID) {
> +		/* The pcpu has been removed */
> +		*result = 0;

You use #defines for the other cases. Should there be
one for 0? Is 0 PCPU_REMOVED?

> +		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;
> +	}
> +
> +
> +	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;

How about #define PCPU_BAD -1?

.. snip..
> +/*
> + * type: 0 add, 1 remove
> + */

Why not make this an enum?

> +int xen_pcpu_hotplug(int type, uint32_t apic_id)