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

From: tj@xxxxxxxxxx
Date: Tue Jul 30 2024 - 15:50:10 EST


Hello, Boy.

On Fri, Jul 26, 2024 at 03:43:27AM +0000, Boy Wu (吳勃誼) wrote:
...
> The use of a spinlock with u64 sync is suggested in
> include/linux/u64_stats_sync.h:33.
>
> * Usage :
> *
> * Stats producer (writer) should use following template granted it
> already got
> * an exclusive access to counters (a lock is already taken, or per cpu
> * data is used [in a non preemptable context])
> *
> * spin_lock_bh(...) or other synchronization to get exclusive access
> * ...
> * u64_stats_update_begin(&stats->syncp);
> * u64_stats_add(&stats->bytes64, len); // non atomic operation
> * u64_stats_inc(&stats->packets64); // non atomic operation
> * u64_stats_update_end(&stats->syncp);
>
> Is this a incorrect statment?

That's not incorrect and it'd make sense if we really want to use u64_sync -
e.g. the reader is hot path. Here, just a spinlock would be simpler and do
fine.

Thanks.

--
tejun