Re: [PATCH v3 1/2] kstats: kernel metric collector

[Paolo Abeni <pabeni@redhat.com>]

Hi,

I'm sorry for the lack of feedback on earlier iteration, I've simply
been too slow.

On Wed, 2020-02-26 at 05:50 -0800, Luigi Rizzo wrote:
[...]
> +static void ks_print(struct seq_file *p, int slot, int cpu, u64 sum,
> +		     u64 tot, unsigned long samples, unsigned long avg)
> +{
> +	unsigned long frac = (tot == 0) ? 0 : ((sum % tot) * 1000000) / tot;

I think/fear kbuildbot will still trigger some build issues on 32bit
arches on the above line.

I think div64_u64_rem() should be used in place of '%' for and 
div64_u64() in place of '/' when operating on 64bits integers. 

There are a few more occurences below.

[...]
> ++	/* preempt_disable makes sure samples and sum modify the same slot.
> +	 * this_cpu_add() uses a non-interruptible add to protect against
> +	 * hardware interrupts which may call kstats_record.
> +	 */
> +	preempt_disable();
> +	this_cpu_add(ks->slots[slot].samples, 1);

I think 'this_cpu_inc()' could be used here.

> +	this_cpu_add(ks->slots[slot].sum,
> +		     bucket < SUM_SCALE ? val : (val >> (bucket - SUM_SCALE)));
> +	preempt_enable();
> +}
> +EXPORT_SYMBOL(kstats_record);

Thanks for sharing, this infra will be likely quite useful to me :)

Cheers,

Paolo