On Wed, Jul 28, 2021 at 5:39 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
> 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.
>
Sure, will inline these values.
Will remove the loghist base, since base 2 log is enough for any
number. No other base is needed.
> >
> > + */
> > +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
