On Wed, 2024-07-17 at 11:44 +0800, Boy.Wu wrote:
>
> I think there is another way to fix it. Maybe we should address the
> root cause, which is that u64_stats_update_begin_irqsave
> and u64_stats_update_end_irqrestore do not protect the seqcount in 32
> bit SMP systems. This can be fix by the following change.
>
> diff --git a/include/linux/u64_stats_sync.h
> b/include/linux/u64_stats_sync.h
> index 46040d66334a..94dd74b4fb2c 100644
> --- a/include/linux/u64_stats_sync.h
> +++ b/include/linux/u64_stats_sync.h
> @@ -64,6 +64,7 @@
> struct u64_stats_sync {
> #if BITS_PER_LONG == 32
> seqcount_t seq;
> + spinlock_t spinlock;
> #endif
> };
>
> @@ -138,6 +139,7 @@ static inline void u64_stats_inc(u64_stats_t *p)
> static inline void u64_stats_init(struct u64_stats_sync *syncp)
> {
> seqcount_init(&syncp->seq);
> + spin_lock_init(&syncp->spinlock)
> }
>
> static inline void __u64_stats_update_begin(struct u64_stats_sync
> *syncp)
> @@ -191,6 +193,7 @@ static inline unsigned long
> u64_stats_update_begin_irqsave(struct u64_stats_sync
> {
> unsigned long flags = __u64_stats_irqsave();
>
> + spin_lock_irq(&syncp->spinlock);
> __u64_stats_update_begin(syncp);
> return flags;
> }
> @@ -199,6 +202,7 @@ static inline void
> u64_stats_update_end_irqrestore(struct u64_stats_sync *syncp,
> unsigned long
> flags)
> {
> __u64_stats_update_end(syncp);
> + spin_unlock_irq(&syncp->spinlock);
> __u64_stats_irqrestore(flags);
> }
>
> --
> boy.wu
>
Never mind, there is a usage in u64_stats_sync.h
* 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);
--
boy.wu
