On Wed, Jul 6, 2016 at 6:26 AM, Tejun Heo <tj@xxxxxxxxxx> wrote: > Hello, > > On Mon, Jul 04, 2016 at 10:46:54PM +0800, Jiale Li wrote: >> These days, we have tested the cgroup blkio throttle function use fio, >> and we found a problem that when we use buffered IO or set the big block >> size like 1M, then the IO performance cannot reach the value we set. >> For example we set blkio.throttle.read_bps_device as 10M, in kernel >> version 4.3 IO performance can only reach 6M, and in kernel version >> 4.4 the actual IO bps is only 3.1M. >> >> Then we did some research and find that in kernel version 4.3 brought in >> blk_queue_split() function to split the big size bio into several parts, >> and some of them are calling the generic_make_request() again, this result >> the bio been throttled more than once. so the actual bio sent to device is >> less than we expected. >> >> We have checked the newest kernel of 4.7-rc5, this problem is still exist. >> >> Based on this kind of situation, we propose a fix solution to add a flag bit >> in bio to let the splited bio bypass the blk_queue_split(). Below is the patch >> we used to fix this problem. > > Thanks a lot for the report. Hmmm... there was another brekage around > split bios, I wonder whether the two can be handled together somehow. > > http://lkml.kernel.org/g/1466583730-28595-1-git-send-email-lars.ellenberg@xxxxxxxxxx > That is another issue, which is a deadlock caused by queuing the remainder bio from splitting before BIOs generated in .make_request_fn(). >> diff --git a/block/blk-merge.c b/block/blk-merge.c >> index 2613531..7b17a65 100644 >> --- a/block/blk-merge.c >> +++ b/block/blk-merge.c >> @@ -190,6 +190,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio, >> >> bio_chain(split, *bio); >> trace_block_split(q, split, (*bio)->bi_iter.bi_sector); >> + bio_set_flag(*bio, BIO_SPLITED); > > Split's past participle form is split, so BIO_SPLIT would be right > here. > >> generic_make_request(*bio); >> *bio = split; >> } >> diff --git a/block/blk-throttle.c b/block/blk-throttle.c >> index 47a3e54..4ffde95 100644 >> --- a/block/blk-throttle.c >> +++ b/block/blk-throttle.c >> @@ -1403,6 +1403,10 @@ bool blk_throtl_bio(struct request_queue *q, struct blkcg_gq *blkg, >> >> WARN_ON_ONCE(!rcu_read_lock_held()); >> >> + /* if the bio has been splited, should not throttle again */ >> + if (bio_flagged(bio, BIO_SPLITED)) >> + goto out; >> + > > But that said, can't we just copy BIO_THROTLED while splitting? > > Thanks. > > -- > tejun Thanks, Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
