On Wed, Jul 28, 2021 at 5:39 AM Paolo Bonzini [off-list ref] wrote:

On 06/07/21 20:03, Jing Zhang wrote:

quoted

+#define LINHIST_SIZE_SMALL           10
+#define LINHIST_SIZE_MEDIUM          20
+#define LINHIST_SIZE_LARGE           50
+#define LINHIST_SIZE_XLARGE          100
+#define LINHIST_BUCKET_SIZE_SMALL    10
+#define LINHIST_BUCKET_SIZE_MEDIUM   100
+#define LINHIST_BUCKET_SIZE_LARGE    1000
+#define LINHIST_BUCKET_SIZE_XLARGE   10000
+
+#define LOGHIST_SIZE_SMALL           8
+#define LOGHIST_SIZE_MEDIUM          16
+#define LOGHIST_SIZE_LARGE           32
+#define LOGHIST_SIZE_XLARGE          64
+#define LOGHIST_BASE_2                       2

I'd prefer inlining all of these.  For log histograms use 2 directly in
STATS_DESC_LOG_HIST, since the update function below uses fls64.

Sure, will inline these values.
Will remove the loghist base, since base 2 log is enough for any
number. No other base is needed.

quoted

+ */
+void kvm_stats_linear_hist_update(u64 *data, size_t size,
+                               u64 value, size_t bucket_size)
+{
+     size_t index = value / bucket_size;
+
+     if (index >= size)
+             index = size - 1;
+     ++data[index];
+}
+

Please make this function always inline, so that the compiler optimizes
the division.

Sure.

Also please use array_index_nospec to clamp the index to the size, in
case value comes from a memory access as well.  Likewise for
kvm_stats_log_hist_update.

Thanks. Will do.

Paolo

Jing