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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux