Re: [PATCH v4] blk-cgroup: Replace u64 sync with spinlock for iostat

Tejun Heo
2024-07-18T21:15:30Z

Hello,

On Thu, Jul 18, 2024 at 04:41:12PM +0800, boy.wu wrote:
> static void blkg_clear_stat(struct blkcg_gq *blkg)
> {
> int cpu;
>
> +#if BITS_PER_LONG == 32
> + guard(raw_spinlock_irqsave)(&blkg_stat_lock);
> +#endif

Can you please collect the ifdefs into a single place? If guard can be used
for that, that's great. If not, just spin_lock/unlock wrappers are fine too,
but please collect them into a single place and add a comment explaining why
this is necessary and why u64_sync isn't being used.

Also, for blkg_clear_stat(), we're running a slight chance of clearing of
iostat_cpu racing against state updates from the hot path. Given the
circumstances - stat reset is an cgroup1-only feature which isn't used
widely and a problematic interface to begin with, I believe this is a valid
choice but it needs to be noted.

Thanks.

--
tejun