Re: [PATCH v9 07/12] timekeeping: add percpu counter for tracking floor swap events

[Thomas Gleixner]

On Wed, Oct 02 2024 at 14:49, Jeff Layton wrote:
> ---
>  fs/inode.c                         |  5 +++--

Grmbl. I explicitely asked to split this into timekeeping and fs
patches, no?

That allows me to pick the timekeeping patches up myself and give
Christian a stable tag to pull them from. That lets me deal with the
conflicts with other timekeeping stuff which is coming up instead of
having cross tree conflicts.

> +unsigned long timekeeping_get_mg_floor_swaps(void)
> +{
> +	int i;
> +	unsigned long sum = 0;

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations

And please use 'cpu'

> +
> +	for_each_possible_cpu(i)
> +		sum += per_cpu(timekeeping_mg_floor_swaps, i);

This needs data_race(per_cpu.....) to tell KCSAN that this is
intentionally racy.

Your previous fs specific patch has the same issue.

> +	return sum < 0 ? 0 : sum;

Right, a sum of unsigned longs really needs to be checked for being negative.

>  #ifdef CONFIG_DEBUG_FS
> +DECLARE_PER_CPU(unsigned long, timekeeping_mg_floor_swaps);
> +static inline void timekeeping_inc_mg_floor_swaps(void)

Did you lose your newline key?. Can we please not glue this together for
readability sake?

Thanks,

        tglx