| |
|
 |
|
 |
|
|
|
| |
|
|
xen-devel
RE: [Xen-devel] RE: [PATCH][DOM0] Expose physical CPU information in dom
Konrad Rzeszutek Wilk wrote:
> 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?
0 means no changes, < 0 means something wrong (like hypercall failure, or
memory allocation failed).
The defined macro is for state changed (i.e. >0). Yes, we can define 0 as
PCPU_NO_CHANGE.
For the code above, if the pcpu does not exist before in kernel, it is 0, means
no change, otherwise it will be set as 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?
-1 means something wrong, not PCPU bad.
>
> .. snip..
>> +/*
>> + * type: 0 add, 1 remove
>> + */
>
> Why not make this an enum?
>
>> +int xen_pcpu_hotplug(int type, uint32_t apic_id)
In fact, there is macro for it already. I give the value here to make it more
clean.
if (!found && (type == HOTPLUG_TYPE_ADD))
printk(KERN_WARNING "The cpu is not added into Xen HV?\n");
if (found && (type == HOTPLUG_TYPE_REMOVE))
printk(KERN_WARNING "The cpu still exits in Xen HV?\n");
return 0;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
| |
|
Home