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

["tj@kernel.org" <tj@kernel.org>]

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