Re: [Xen-devel] [PATCH] introduce and used relaxed cpumask operations

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.01.15 at 13:21, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 19/01/15 15:58, Jan Beulich wrote:
>> --- a/xen/common/core_parking.c
>> +++ b/xen/common/core_parking.c
>> @@ -75,11 +75,10 @@ static unsigned int core_parking_perform
>>              if ( core_weight < core_tmp )
>>              {
>>                  core_weight = core_tmp;
>> -                cpumask_clear(&core_candidate_map);
>> -                cpumask_set_cpu(cpu, &core_candidate_map);
>> +                cpumask_copy(&core_candidate_map, cpumask_of(cpu));
> 
> It is probably worth mentioning changes like this in the commit message,
> as they are slightly more than just a simple removal of the lock prefix.

Added.

>> +static inline void __cpumask_clr_cpu(int cpu, cpumask_t *dstp)
>> +{
>> +    __clear_bit(cpumask_check(cpu), dstp->bits);
>> +}
>> +
> 
> While I can appreciate the want for a shorter function name, I feel that
> consistency with its locked alternative is more important.

I sort of expected a comment to that effect, but decided to use the
shorter names nevertheless. Let's see what others, namely the REST
maintainers, say.

>> -static inline int cpumask_test_and_set_cpu(int cpu, cpumask_t *addr)
>> +static inline int cpumask_test_and_set_cpu(int cpu, volatile cpumask_t 
>> *addr)
>>  {
>>      return test_and_set_bit(cpumask_check(cpu), addr->bits);
>>  }
>>  
>> -static inline int cpumask_test_and_clear_cpu(int cpu, cpumask_t *addr)
>> +static inline int __cpumask_tst_set_cpu(int cpu, cpumask_t *addr)
>> +{
>> +    return __test_and_set_bit(cpumask_check(cpu), addr->bits);
>> +}
>> +
>> +static inline int cpumask_test_and_clear_cpu(int cpu, volatile cpumask_t 
>> *addr)
> 
> This introduction of volatile also need mentioning in the commit
> message, but I would agree that it should be here.

Done too.

Jan


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