Re: [Xen-devel] [PATCH 1/2] core-parking: interact with runtime SMT-disabling

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.04.19 at 13:51, <julien.grall@xxxxxxx> wrote:
> On 11/04/2019 13:45, Jan Beulich wrote:
>> --- a/xen/common/core_parking.c
>> +++ b/xen/common/core_parking.c
> 
> 
> [...]
> 
>> +bool core_parking_remove(unsigned int cpu)
> 
> Something looks wrong. This function is implemented in common code but...
> 
>> +{
>> +    unsigned int i;
>> +    bool found = false;
>> +
>> +    spin_lock(&accounting_lock);
>> +
>> +    for ( i = 0; i < cur_idle_nums; ++i )
>> +        if ( core_parking_cpunum[i] == cpu )
>> +        {
>> +            found = true;
>> +            --cur_idle_nums;
>> +            break;
>> +        }
>> +
>> +    for ( ; i < cur_idle_nums; ++i )
>> +        core_parking_cpunum[i] = core_parking_cpunum[i + 1];
>> +
>> +    spin_unlock(&accounting_lock);
>> +
>> +    return found;
>> +}
>> +
>>   uint32_t get_cur_idle_nums(void)
>>   {
>>       return cur_idle_nums;
>> --- a/xen/include/asm-x86/smp.h
>> +++ b/xen/include/asm-x86/smp.h
>> @@ -63,6 +63,7 @@ long cpu_up_helper(void *data);
>>   long cpu_down_helper(void *data);
>>   
>>   long core_parking_helper(void *data);
>> +bool core_parking_remove(unsigned int cpu);
> 
> The prototype is declared in asm-x86/smp.h. Actually, it seems all the 
> function 
> exported in core_parking.c have their prototype declared in asm-x86/smp.h.

I've noticed this too, but this is not an uncommon thing (hence
the CORE_PARKING Kconfig setting). Various such instance
have been eliminated since the introduction of Arm support,
but others remain. It's not the purpose of this series to clean
up this aspect, or ...

> This raises the question of whether it makes sense to have core_parking.c in 
> common. If it makes sense, then the prototype should be declared in 
> include/xen.

... to decide whether the .c file or the declarations should move.
Personally I think this is generic enough a concept that the .c
should remain where it is, in which case we'd likely want to gain
a xen/core-parking.h at some point.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel