brookxu wrote on 2022/1/7 14:50:
>
>
> Ming Lei wrote on 2022/1/7 11:52:
>> On Thu, Jan 06, 2022 at 03:46:54PM +0800, brookxu wrote:
>>> Hi Ming:
>>>
>>> I think it may be due to other reasons, I test this patch seems work fine,
>>> Can you verify it in your environment?
>>
>> Your patch can't cover the issue Ning Li reported.
>>
>>>
>>>
>>> From 2c7305042e71d0f53ca50a8a3f2eebe6a2dcdb86 Mon Sep 17 00:00:00 2001
>>> From: Chunguang Xu <brookxu@xxxxxxxxxxx>
>>> Date: Thu, 6 Jan 2022 15:18:50 +0800
>>> Subject: [PATCH] blk-throtl: avoid double charge of bio IOPS due to split
>>>
>>> After commit 900e08075202("block: move queue enter logic into
>>> blk_mq_submit_bio()"), submit_bio_checks() is moved from the
>>> entrance of the IO stack to the specific submit_bio() entrance.
>>> Therefore, the IO may be splited before entering blk-throtl, so
>>> we need to check whether the BIO is throttled, and only need
>>> to update the io_split_cnt for the throttled bio to avoid
>>> double charge.
>>
>> Actually since commit 900e08075202, your commit 4f1e9630afe6
>> ("blk-throtl: optimize IOPS throttle for large IO scenarios") doesn't
>> need any more, because split bio is always sent to submit_bio_checks().
> This patch should still be necessary for those devices whose IO needs to go
> through __submit_bio_fops(). If the IO is split after submit_bio_checks(),
> the cloned IO will be marked with BIO_THROTTLED and will not be charged again
> when resubmitted.
Excuse me. Think deeply, we should sitll need to add the following judgment?
This maybe more reliable, because we only need to add the IOPS count for
THROTTLED IO that was missed due to split. It can also avoid double charge
problems in the future. Any comments is required.
@@ -2049,6 +2049,9 @@ void blk_throtl_charge_bio_split(struct bio *bio)
struct throtl_service_queue *parent_sq;
bool rw = bio_data_dir(bio);
+ if (!bio_flagged(bio, BIO_THROTTLED))
+ return;
+
do {
if (!parent->has_rules[rw])
break;
>> But I don't think that way is reasonable, especially precise driver tag
>> and request is consumed by each throttling, so the following patch is posted:
>
> Right.
>
>>
>> https://lore.kernel.org/linux-block/20220104134223.590803-1-ming.lei@xxxxxxxxxx/T/#u
>>
>>
>> Thanks,
>> Ming
>>
