On Tue, 2024-07-30 at 09:49 -1000, tj@kernel.org wrote:

 	 
External email : Please do not click links or open attachments until
you have verified the sender or the content.
 Hello, Boy.

On Fri, Jul 26, 2024 at 03:43:27AM +0000, Boy Wu (吳勃誼) wrote:
...

quoted

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

quoted

 * data is used [in a non preemptable context])
 *
 *   spin_lock_bh(...) or other synchronization to get exclusive

access

quoted

 *   ...
 *   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

u64_sync with spin lock has the benefit of locking only when writing
iostat, but replacing u64_sync with spin lock will lock not only when
writing iostat but also when reading iostat. Does it have enough
benefit to replace u64_sync and add the cost of locking when reading
iostat?

--
Boy.Wu