Re: [PATCH kvm-unit-tests] api: verify number of dirty pages

[Bandan Das <bsd@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  api/dirty-log-perf.cc |  6 +++---
>  api/memmap.cc         | 15 ++++++++++++++-
>  api/memmap.hh         |  2 +-
>  3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/api/dirty-log-perf.cc b/api/dirty-log-perf.cc
> index 89043b0..f87b4b4 100644
> --- a/api/dirty-log-perf.cc
> +++ b/api/dirty-log-perf.cc
> @@ -52,11 +52,11 @@ void check_dirty_log(kvm::vcpu& vcpu, mem_slot& slot, void* slot_head)
>          do_guest_write(vcpu, slot_head, i, nr_slot_pages);
>  
>          uint64_t start_ns = time_ns();
> -        slot.update_dirty_log();
> +        int n = slot.update_dirty_log();
>          uint64_t end_ns = time_ns();
>  
> -        printf("get dirty log: %10lld ns for %10lld dirty pages\n",
> -               end_ns - start_ns, i);
> +        printf("get dirty log: %10lld ns for %10d dirty pages (expected %lld)\n",
> +               end_ns - start_ns, n, i);
>      }
>  
>      slot.set_dirty_logging(false);
> diff --git a/api/memmap.cc b/api/memmap.cc
> index 041e84a..cf44824 100644
> --- a/api/memmap.cc
> +++ b/api/memmap.cc
> @@ -1,5 +1,6 @@
>  
>  #include "memmap.hh"
> +#include <numeric>
>  
>  mem_slot::mem_slot(mem_map& map, uint64_t gpa, uint64_t size, void* hva)
>      : _map(map)
> @@ -60,9 +61,21 @@ bool mem_slot::dirty_logging() const
>      return _dirty_log_enabled;
>  }
>  
> -void mem_slot::update_dirty_log()
> +static inline int hweight(uint64_t w)
> +{
> +    w -= (w >> 1) & 0x5555555555555555;
> +    w =  (w & 0x3333333333333333) + ((w >> 2) & 0x3333333333333333);
> +    w =  (w + (w >> 4)) & 0x0f0f0f0f0f0f0f0f;
> +    return (w * 0x0101010101010101) >> 56;
> +}
> +
> +int mem_slot::update_dirty_log()
>  {
>      _map._vm.get_dirty_log(_slot, &_log[0]);
> +    return std::accumulate(_log.begin(), _log.end(), 0,
> +                           [] (int prev, ulong elem) -> int {
> +                               return prev + hweight(elem);
> +                           });
>  }
>  
>  bool mem_slot::is_dirty(uint64_t gpa) const
> diff --git a/api/memmap.hh b/api/memmap.hh
> index 59ec619..48711ae 100644
> --- a/api/memmap.hh
> +++ b/api/memmap.hh
> @@ -15,7 +15,7 @@ public:
>      ~mem_slot();
>      void set_dirty_logging(bool enabled);
>      bool dirty_logging() const;
> -    void update_dirty_log();
> +    int update_dirty_log();
>      bool is_dirty(uint64_t gpa) const;
>  private:
>      void update();

Can the compiler builtin be used instead of hweight ?
Nevertheless, this change makes sense.

Reviewed-by: Bandan Das <bsd@redhat.com>