On 22/01/2018 09:39, Jan Beulich wrote: >>>> On 19.01.18 at 18:48, <[email protected]> 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 <[email protected]> >> 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 [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
