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