Re: [PATCH v1 1/4] KVM: stats: Support linear and logarithmic histogram statistics

[Paolo Bonzini <pbonzini@redhat.com>]

On 06/07/21 20:03, Jing Zhang wrote:
> +#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.

> 
> + */
> +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.

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.

Paolo