Re: [Xen-devel] [PATCH 4/6] replace vCPU's dirty CPU mask by numeric ID

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/01/2018 09:39, Jan Beulich wrote:
>>>> On 19.01.18 at 18:48, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 19/01/18 16:06, Jan Beulich wrote:
>>> At most one bit can be set in the masks, so especially on larger systems
>>> it's quite a bit of unnecessary memory and processing overhead to track
>>> the information as a mask. Store the numeric ID of the respective CPU
>>> instead, or NR_CPUS if no dirty state exists.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Definitely +1 for this change.
>>
>> However, the comparison against nr_cpu_ids isn't completely obvious as
>> to its function.  How about introducing a predicate such as vcpu_dirty()
>> which wraps the use of the id?
> I can do that.
>
>> Also, you'd get better code by using NR_CPUS which is a compile-time
>> constant, rather than nr_cpu_ids which would be a memory read.
> I did consider it, but decided that the check being more tight when
> using nr_cpu_ids is preferable. The checks sit in ASSERT()s only
> anyway, i.e. I don't think performance matters much.

The problem is not performance of the assertions, but rather how obvious
the code is concerning its function.  Mixing the use of nr_cpu_ids and
NR_CPUS makes things less obvious.

Also, on further consideration, a #define VCPU_CLEAN (~0u) constant
might be better than NR_CPUS for comparisons, as it can be used as a
imm8 rather than needing to be imm32, and would certainly help the
clarity of the logic.

~Andrew

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