Re: [PATCH v2] block/blk-iocost: read ioc_pd_stat params inside ioc->lock

Tao Cui <[email protected]> Tue, 4 Aug 2026 11:04:10 +0800
Newsgroups org.kernel.vger.cgroups,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

在 2026/8/4 00:47, Tejun Heo 写道:
> On Mon, Aug 03, 2026 at 09:22:01PM +0800, Tao Cui wrote:
>> @@ -3092,9 +3092,12 @@ static void ioc_pd_stat(struct blkg_policy_data *pd, struct seq_file *s)
>>  {
>>  	struct ioc_gq *iocg = pd_to_iocg(pd);
>>  	struct ioc *ioc = iocg->ioc;
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&ioc->lock, flags);
>>  
>>  	if (!ioc->enabled)
>> -		return;
>> +		goto out;
>>  
>>  	if (iocg->level == 0) {
>>  		unsigned vp10k = DIV64_U64_ROUND_CLOSEST(
>> @@ -3110,6 +3113,8 @@ static void ioc_pd_stat(struct blkg_policy_data *pd, struct seq_file *s)
>>  			iocg->last_stat.wait_us,
>>  			iocg->last_stat.indebt_us,
>>  			iocg->last_stat.indelay_us);
>> +out:
>> +	spin_unlock_irqrestore(&ioc->lock, flags);
>>  }
> 
> There's nothing in this function that requires synchronized reads. Might as
> well just annotate them with data_race(). Otherwise, this can lead to a lot
> of lock operations when there are a lot of cgroups and high frequency stat
> reads.
> 
Agreed, there's nothing in this function that requires synchronized
reads, these are all stat values where stale reads are harmless.
Will switch to data_race() in v3.

Thanks,
Tao

> Thanks.
>