On 2/22/25 9:13 AM, Yunsheng Lin wrote:
> On 2025/2/21 19:52, Sebastian Andrzej Siewior wrote:
>> @@ -99,11 +106,19 @@ bool page_pool_get_stats(const struct page_pool *pool,
>> const struct page_pool_recycle_stats *pcpu =
>> per_cpu_ptr(pool->recycle_stats, cpu);
>>
>> - stats->recycle_stats.cached += pcpu->cached;
>> - stats->recycle_stats.cache_full += pcpu->cache_full;
>> - stats->recycle_stats.ring += pcpu->ring;
>> - stats->recycle_stats.ring_full += pcpu->ring_full;
>> - stats->recycle_stats.released_refcnt += pcpu->released_refcnt;
>> + do {
>> + start = u64_stats_fetch_begin(&pcpu->syncp);
>> + u64_stats_add(&stats->recycle_stats.cached,
>> + u64_stats_read(&pcpu->cached));
>> + u64_stats_add(&stats->recycle_stats.cache_full,
>> + u64_stats_read(&pcpu->cache_full));
>> + u64_stats_add(&stats->recycle_stats.ring,
>> + u64_stats_read(&pcpu->ring));
>> + u64_stats_add(&stats->recycle_stats.ring_full,
>> + u64_stats_read(&pcpu->ring_full));
>> + u64_stats_add(&stats->recycle_stats.released_refcnt,
>> + u64_stats_read(&pcpu->released_refcnt));
>
> It seems the above u64_stats_add() may be called more than one time
> if the below u64_stats_fetch_retry() returns true, which might mean
> the stats is added more than it is needed.
>
> It seems more correct to me that pool->alloc_stats is read into a
> local varible in the while loop and then do the addition outside
> the while loop?
I also think the above code is incorrect, and local variables to store
the read stats are needed.
/P
