Re: [Xen-devel] [PATCH v5 1/6] xen: Add convenient macro boot_cpu

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/05/14 17:04, Jan Beulich wrote:
>>>> On 06.05.14 at 17:58, <julien.grall@xxxxxxxxxx> wrote:
>> On 05/05/2014 08:59 AM, Jan Beulich wrote:
>>>>>> On 02.05.14 at 17:52, <julien.grall@xxxxxxxxxx> wrote:
>>>> --- a/xen/include/xen/percpu.h
>>>> +++ b/xen/include/xen/percpu.h
>>>> @@ -16,6 +16,9 @@
>>>>  /* Preferred on Xen. Also see arch-defined per_cpu(). */
>>>>  #define this_cpu(var)    __get_cpu_var(var)
>>>>  
>>>> +/* Access variable on boot CPU */
>>>> +#define boot_cpu(var)   per_cpu(var, 0)
>>>> +
>>>>  /* Linux compatibility. */
>>>>  #define get_cpu_var(var) this_cpu(var)
>>>>  #define put_cpu_var(var)
>>> I can only second Andrew's comment - without it becoming clear when
>>> and why this would be used, this isn't going to be acceptable (and if
>>> it really was correct and worthwhile, this small a change wouldn't need
>>> separating out from the code that's intended to actually make use of it).
>> It's used in the next patch (ie #2). I thought it was better to move out
>> this small change rather than introducing it in the patch.
>>
>> This macro is here for a shortcut of per_cpu(myvar, 0). We have few
>> usage on ARM, most of them are during boot.
>>
>> There is another when a secondary CPUs is booting. We need to retrieve
>> the PPIs type (level/edge...) from the boot CPU.
>>
>> As said in the answer to Andrew, I can either move this macro in an ARM
>> specific header or open code per_cpu(...,0).
> Either is fine with me, but putting this in common code isn't.
>
> Jan

Agreed - this should not be common.

If you eventually want to support hotplug, making an arm specific
boot_cpu() might help those efforts in the future.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel