| Message ID | 20200605141629.15347-3-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bfq: Two fixes and a cleanup for sequential readers | expand |
> Il giorno 5 giu 2020, alle ore 16:16, Jan Kara <jack@suse.cz> ha scritto: > > Currently whenever bfq queue has a request queued we add now - > last_completion_time to the think time statistics. This is however > misleading in case the process is able to submit several requests in > parallel because e.g. if the queue has request completed at time T0 and > then queues new requests at times T1, T2, then we will add T1-T0 and > T2-T0 to think time statistics which just doesn't make any sence (the > queue's think time is penalized by the queue being able to submit more > IO). I've thought about this issue several times. My concern is that, by updating the think time only when strictly meaningful, we reduce the number of samples. In some cases, we may reduce it to a very low value. For this reason, so far I have desisted from changing the current scheme. IOW, I opted for dirtier statistics to avoid the risk of too scarse statistics. Any feedback is very welcome. Thanks, Paolo > So add to think time statistics only time intervals when the queue > had no IO pending. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > block/bfq-iosched.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c > index c66c3eaa9e26..4b1c9c5f57b6 100644 > --- a/block/bfq-iosched.c > +++ b/block/bfq-iosched.c > @@ -5192,8 +5192,16 @@ static void bfq_update_io_thinktime(struct bfq_data *bfqd, > struct bfq_queue *bfqq) > { > struct bfq_ttime *ttime = &bfqq->ttime; > - u64 elapsed = ktime_get_ns() - bfqq->ttime.last_end_request; > - > + u64 elapsed; > + > + /* > + * We are really interested in how long it takes for the queue to > + * become busy when there is no outstanding IO for this queue. So > + * ignore cases when the bfq queue has already IO queued. > + */ > + if (bfqq->dispatched || bfq_bfqq_busy(bfqq)) > + return; > + elapsed = ktime_get_ns() - bfqq->ttime.last_end_request; > elapsed = min_t(u64, elapsed, 2ULL * bfqd->bfq_slice_idle); > > ttime->ttime_samples = (7*ttime->ttime_samples + 256) / 8; > -- > 2.16.4 >
On Thu 11-06-20 16:11:10, Paolo Valente wrote: > > > > Il giorno 5 giu 2020, alle ore 16:16, Jan Kara <jack@suse.cz> ha scritto: > > > > Currently whenever bfq queue has a request queued we add now - > > last_completion_time to the think time statistics. This is however > > misleading in case the process is able to submit several requests in > > parallel because e.g. if the queue has request completed at time T0 and > > then queues new requests at times T1, T2, then we will add T1-T0 and > > T2-T0 to think time statistics which just doesn't make any sence (the > > queue's think time is penalized by the queue being able to submit more > > IO). > > I've thought about this issue several times. My concern is that, by > updating the think time only when strictly meaningful, we reduce the > number of samples. In some cases, we may reduce it to a very low > value. For this reason, so far I have desisted from changing the > current scheme. IOW, I opted for dirtier statistics to avoid the risk > of too scarse statistics. Any feedback is very welcome. I understand the concern. But: a) I don't think adding these samples to statistics helps in any way (you cannot improve the prediction power of the statistics by including in it some samples that are not directly related to the thing you try to predict). And think time is used to predict the answer to the question: If bfq queue becomes idle, how long will it take for new request to arrive? So second and further requests are simply irrelevant. b) From my testing with 4 fio sequential readers (the workload mentioned in the previous patch) this patch caused a noticeable change in computed think time and that allowed fio processes to be reliably marked as having short think time (without this patch they were oscilating between short ttime and not short ttime) and consequently achieving better throughput because we were idling for new requests from these bfq queues. Note that this was with somewhat aggressive slice_idle setting (2ms). Having slice_idle >= 4ms was enough hide the ttime computation issue but still this demonstrates that these bogus samples noticeably raise computed average. Honza > > So add to think time statistics only time intervals when the queue > > had no IO pending. > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > --- > > block/bfq-iosched.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c > > index c66c3eaa9e26..4b1c9c5f57b6 100644 > > --- a/block/bfq-iosched.c > > +++ b/block/bfq-iosched.c > > @@ -5192,8 +5192,16 @@ static void bfq_update_io_thinktime(struct bfq_data *bfqd, > > struct bfq_queue *bfqq) > > { > > struct bfq_ttime *ttime = &bfqq->ttime; > > - u64 elapsed = ktime_get_ns() - bfqq->ttime.last_end_request; > > - > > + u64 elapsed; > > + > > + /* > > + * We are really interested in how long it takes for the queue to > > + * become busy when there is no outstanding IO for this queue. So > > + * ignore cases when the bfq queue has already IO queued. > > + */ > > + if (bfqq->dispatched || bfq_bfqq_busy(bfqq)) > > + return; > > + elapsed = ktime_get_ns() - bfqq->ttime.last_end_request; > > elapsed = min_t(u64, elapsed, 2ULL * bfqd->bfq_slice_idle); > > > > ttime->ttime_samples = (7*ttime->ttime_samples + 256) / 8; > > -- > > 2.16.4 > > >
> Il giorno 11 giu 2020, alle ore 16:39, Jan Kara <jack@suse.cz> ha scritto: > > On Thu 11-06-20 16:11:10, Paolo Valente wrote: >> >> >>> Il giorno 5 giu 2020, alle ore 16:16, Jan Kara <jack@suse.cz> ha scritto: >>> >>> Currently whenever bfq queue has a request queued we add now - >>> last_completion_time to the think time statistics. This is however >>> misleading in case the process is able to submit several requests in >>> parallel because e.g. if the queue has request completed at time T0 and >>> then queues new requests at times T1, T2, then we will add T1-T0 and >>> T2-T0 to think time statistics which just doesn't make any sence (the >>> queue's think time is penalized by the queue being able to submit more >>> IO). >> >> I've thought about this issue several times. My concern is that, by >> updating the think time only when strictly meaningful, we reduce the >> number of samples. In some cases, we may reduce it to a very low >> value. For this reason, so far I have desisted from changing the >> current scheme. IOW, I opted for dirtier statistics to avoid the risk >> of too scarse statistics. Any feedback is very welcome. > > I understand the concern. Hi, sorry for the sidereal delay. > But: > > a) I don't think adding these samples to statistics helps in any way (you > cannot improve the prediction power of the statistics by including in it > some samples that are not directly related to the thing you try to > predict). And think time is used to predict the answer to the question: If > bfq queue becomes idle, how long will it take for new request to arrive? So > second and further requests are simply irrelevant. > Yes, you are super right in theory. Unfortunately this may not mean that your patch will do only good, for the concerns in my previous email. So, here is a proposal to move forward: 1) I test your patch on my typical set of latency/guaranteed-bandwidth/total-throughput benchmarks 2) You test your patch on a significant set of benchmarks in mmtests What do you think? Thanks, Paolo > b) From my testing with 4 fio sequential readers (the workload mentioned in > the previous patch) this patch caused a noticeable change in computed think > time and that allowed fio processes to be reliably marked as having short > think time (without this patch they were oscilating between short ttime and > not short ttime) and consequently achieving better throughput because we > were idling for new requests from these bfq queues. Note that this was with > somewhat aggressive slice_idle setting (2ms). Having slice_idle >= 4ms was > enough hide the ttime computation issue but still this demonstrates that > these bogus samples noticeably raise computed average. > > Honza > >>> So add to think time statistics only time intervals when the queue >>> had no IO pending. >>> >>> Signed-off-by: Jan Kara <jack@suse.cz> >>> --- >>> block/bfq-iosched.c | 12 ++++++++++-- >>> 1 file changed, 10 insertions(+), 2 deletions(-) >>> >>> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c >>> index c66c3eaa9e26..4b1c9c5f57b6 100644 >>> --- a/block/bfq-iosched.c >>> +++ b/block/bfq-iosched.c >>> @@ -5192,8 +5192,16 @@ static void bfq_update_io_thinktime(struct bfq_data *bfqd, >>> struct bfq_queue *bfqq) >>> { >>> struct bfq_ttime *ttime = &bfqq->ttime; >>> - u64 elapsed = ktime_get_ns() - bfqq->ttime.last_end_request; >>> - >>> + u64 elapsed; >>> + >>> + /* >>> + * We are really interested in how long it takes for the queue to >>> + * become busy when there is no outstanding IO for this queue. So >>> + * ignore cases when the bfq queue has already IO queued. >>> + */ >>> + if (bfqq->dispatched || bfq_bfqq_busy(bfqq)) >>> + return; >>> + elapsed = ktime_get_ns() - bfqq->ttime.last_end_request; >>> elapsed = min_t(u64, elapsed, 2ULL * bfqd->bfq_slice_idle); >>> >>> ttime->ttime_samples = (7*ttime->ttime_samples + 256) / 8; >>> -- >>> 2.16.4 >>> >> > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
On Wed 22-07-20 11:13:28, Paolo Valente wrote: > > a) I don't think adding these samples to statistics helps in any way (you > > cannot improve the prediction power of the statistics by including in it > > some samples that are not directly related to the thing you try to > > predict). And think time is used to predict the answer to the question: If > > bfq queue becomes idle, how long will it take for new request to arrive? So > > second and further requests are simply irrelevant. > > > > Yes, you are super right in theory. > > Unfortunately this may not mean that your patch will do only good, for > the concerns in my previous email. > > So, here is a proposal to move forward: > 1) I test your patch on my typical set of > latency/guaranteed-bandwidth/total-throughput benchmarks > 2) You test your patch on a significant set of benchmarks in mmtests > > What do you think? Sure, I will queue runs for the patches with mmtests :). Honza
On Mon 27-07-20 09:35:15, Jan Kara wrote: > On Wed 22-07-20 11:13:28, Paolo Valente wrote: > > > a) I don't think adding these samples to statistics helps in any way (you > > > cannot improve the prediction power of the statistics by including in it > > > some samples that are not directly related to the thing you try to > > > predict). And think time is used to predict the answer to the question: If > > > bfq queue becomes idle, how long will it take for new request to arrive? So > > > second and further requests are simply irrelevant. > > > > > > > Yes, you are super right in theory. > > > > Unfortunately this may not mean that your patch will do only good, for > > the concerns in my previous email. > > > > So, here is a proposal to move forward: > > 1) I test your patch on my typical set of > > latency/guaranteed-bandwidth/total-throughput benchmarks > > 2) You test your patch on a significant set of benchmarks in mmtests > > > > What do you think? > > Sure, I will queue runs for the patches with mmtests :). Sorry it took so long but I've hit a couple of technical snags when running these tests (plus I was on vacation). So I've run the tests on 4 machines. 2 with rotational disks with NCQ, 2 with SATA SSD. Results are mostly neutral, details are below. For dbench, it seems to be generally neutral but the patches do fix occasional weird outlier which are IMO caused exactly by bugs in the heuristics I'm fixing. Things like (see the outlier for 4 clients with vanilla kernel): vanilla bfq-waker-fixes Amean 1 32.57 ( 0.00%) 32.10 ( 1.46%) Amean 2 34.73 ( 0.00%) 34.68 ( 0.15%) Amean 4 199.74 ( 0.00%) 45.76 ( 77.09%) Amean 8 65.41 ( 0.00%) 65.47 ( -0.10%) Amean 16 95.46 ( 0.00%) 96.61 ( -1.21%) Amean 32 148.07 ( 0.00%) 147.66 ( 0.27%) Amean 64 291.17 ( 0.00%) 289.44 ( 0.59%) For pgbench and bonnie, patches are neutral for all the machines. For reaim disk workload, patches are mostly neutral, just on one machine with SSD they seem to improve XFS results and worsen ext4 results. But results look rather noisy on that machine so it may be just a noise... For parallel dd(1) processes reading from multiple files, results are also neutral all machines. For parallel dd(1) processes reading from a common file, results are also neutral except for one machine with SSD on XFS (ext4 was fine) where there seems to be consistent regression for 4 and more processes: vanilla bfq-waker-fixes Amean 1 393.30 ( 0.00%) 391.02 ( 0.58%) Amean 4 443.88 ( 0.00%) 517.16 ( -16.51%) Amean 7 599.60 ( 0.00%) 748.68 ( -24.86%) Amean 12 1134.26 ( 0.00%) 1255.62 ( -10.70%) Amean 21 1940.50 ( 0.00%) 2206.29 ( -13.70%) Amean 30 2381.08 ( 0.00%) 2735.69 ( -14.89%) Amean 48 2754.36 ( 0.00%) 3258.93 ( -18.32%) I'll try to reproduce this regression and check what's happening... So what do you think, are you fine with merging my patches now? Honza
On Wed 26-08-20 15:54:19, Jan Kara wrote: > On Mon 27-07-20 09:35:15, Jan Kara wrote: > > On Wed 22-07-20 11:13:28, Paolo Valente wrote: > > > > a) I don't think adding these samples to statistics helps in any way (you > > > > cannot improve the prediction power of the statistics by including in it > > > > some samples that are not directly related to the thing you try to > > > > predict). And think time is used to predict the answer to the question: If > > > > bfq queue becomes idle, how long will it take for new request to arrive? So > > > > second and further requests are simply irrelevant. > > > > > > > > > > Yes, you are super right in theory. > > > > > > Unfortunately this may not mean that your patch will do only good, for > > > the concerns in my previous email. > > > > > > So, here is a proposal to move forward: > > > 1) I test your patch on my typical set of > > > latency/guaranteed-bandwidth/total-throughput benchmarks > > > 2) You test your patch on a significant set of benchmarks in mmtests > > > > > > What do you think? > > > > Sure, I will queue runs for the patches with mmtests :). > > Sorry it took so long but I've hit a couple of technical snags when running > these tests (plus I was on vacation). So I've run the tests on 4 machines. > 2 with rotational disks with NCQ, 2 with SATA SSD. Results are mostly > neutral, details are below. > > For dbench, it seems to be generally neutral but the patches do fix > occasional weird outlier which are IMO caused exactly by bugs in the > heuristics I'm fixing. Things like (see the outlier for 4 clients > with vanilla kernel): > > vanilla bfq-waker-fixes > Amean 1 32.57 ( 0.00%) 32.10 ( 1.46%) > Amean 2 34.73 ( 0.00%) 34.68 ( 0.15%) > Amean 4 199.74 ( 0.00%) 45.76 ( 77.09%) > Amean 8 65.41 ( 0.00%) 65.47 ( -0.10%) > Amean 16 95.46 ( 0.00%) 96.61 ( -1.21%) > Amean 32 148.07 ( 0.00%) 147.66 ( 0.27%) > Amean 64 291.17 ( 0.00%) 289.44 ( 0.59%) > > For pgbench and bonnie, patches are neutral for all the machines. > > For reaim disk workload, patches are mostly neutral, just on one machine > with SSD they seem to improve XFS results and worsen ext4 results. But > results look rather noisy on that machine so it may be just a noise... > > For parallel dd(1) processes reading from multiple files, results are also > neutral all machines. > > For parallel dd(1) processes reading from a common file, results are also > neutral except for one machine with SSD on XFS (ext4 was fine) where there > seems to be consistent regression for 4 and more processes: > > vanilla bfq-waker-fixes > Amean 1 393.30 ( 0.00%) 391.02 ( 0.58%) > Amean 4 443.88 ( 0.00%) 517.16 ( -16.51%) > Amean 7 599.60 ( 0.00%) 748.68 ( -24.86%) > Amean 12 1134.26 ( 0.00%) 1255.62 ( -10.70%) > Amean 21 1940.50 ( 0.00%) 2206.29 ( -13.70%) > Amean 30 2381.08 ( 0.00%) 2735.69 ( -14.89%) > Amean 48 2754.36 ( 0.00%) 3258.93 ( -18.32%) > > I'll try to reproduce this regression and check what's happening... > > So what do you think, are you fine with merging my patches now? Paolo, any results from running your tests for these patches? I'd like to get these mostly obvious things merged so that we can move on... Honza
> Il giorno 2 set 2020, alle ore 17:17, Jan Kara <jack@suse.cz> ha scritto: > > On Wed 26-08-20 15:54:19, Jan Kara wrote: >> On Mon 27-07-20 09:35:15, Jan Kara wrote: >>> On Wed 22-07-20 11:13:28, Paolo Valente wrote: >>>>> a) I don't think adding these samples to statistics helps in any way (you >>>>> cannot improve the prediction power of the statistics by including in it >>>>> some samples that are not directly related to the thing you try to >>>>> predict). And think time is used to predict the answer to the question: If >>>>> bfq queue becomes idle, how long will it take for new request to arrive? So >>>>> second and further requests are simply irrelevant. >>>>> >>>> >>>> Yes, you are super right in theory. >>>> >>>> Unfortunately this may not mean that your patch will do only good, for >>>> the concerns in my previous email. >>>> >>>> So, here is a proposal to move forward: >>>> 1) I test your patch on my typical set of >>>> latency/guaranteed-bandwidth/total-throughput benchmarks >>>> 2) You test your patch on a significant set of benchmarks in mmtests >>>> >>>> What do you think? >>> >>> Sure, I will queue runs for the patches with mmtests :). >> >> Sorry it took so long but I've hit a couple of technical snags when running >> these tests (plus I was on vacation). So I've run the tests on 4 machines. >> 2 with rotational disks with NCQ, 2 with SATA SSD. Results are mostly >> neutral, details are below. >> >> For dbench, it seems to be generally neutral but the patches do fix >> occasional weird outlier which are IMO caused exactly by bugs in the >> heuristics I'm fixing. Things like (see the outlier for 4 clients >> with vanilla kernel): >> >> vanilla bfq-waker-fixes >> Amean 1 32.57 ( 0.00%) 32.10 ( 1.46%) >> Amean 2 34.73 ( 0.00%) 34.68 ( 0.15%) >> Amean 4 199.74 ( 0.00%) 45.76 ( 77.09%) >> Amean 8 65.41 ( 0.00%) 65.47 ( -0.10%) >> Amean 16 95.46 ( 0.00%) 96.61 ( -1.21%) >> Amean 32 148.07 ( 0.00%) 147.66 ( 0.27%) >> Amean 64 291.17 ( 0.00%) 289.44 ( 0.59%) >> >> For pgbench and bonnie, patches are neutral for all the machines. >> >> For reaim disk workload, patches are mostly neutral, just on one machine >> with SSD they seem to improve XFS results and worsen ext4 results. But >> results look rather noisy on that machine so it may be just a noise... >> >> For parallel dd(1) processes reading from multiple files, results are also >> neutral all machines. >> >> For parallel dd(1) processes reading from a common file, results are also >> neutral except for one machine with SSD on XFS (ext4 was fine) where there >> seems to be consistent regression for 4 and more processes: >> >> vanilla bfq-waker-fixes >> Amean 1 393.30 ( 0.00%) 391.02 ( 0.58%) >> Amean 4 443.88 ( 0.00%) 517.16 ( -16.51%) >> Amean 7 599.60 ( 0.00%) 748.68 ( -24.86%) >> Amean 12 1134.26 ( 0.00%) 1255.62 ( -10.70%) >> Amean 21 1940.50 ( 0.00%) 2206.29 ( -13.70%) >> Amean 30 2381.08 ( 0.00%) 2735.69 ( -14.89%) >> Amean 48 2754.36 ( 0.00%) 3258.93 ( -18.32%) >> >> I'll try to reproduce this regression and check what's happening... >> >> So what do you think, are you fine with merging my patches now? > > Paolo, any results from running your tests for these patches? I'd like to > get these mostly obvious things merged so that we can move on... > Hi, sorry again for my delay. Tested this too, at last. No regression. So gladly Acked-by: Paolo Valente <paolo.valente@linaro.org> And thank you very much for your contributions and patience, Paolo > Honza > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
On Sun 10-01-21 10:23:47, Paolo Valente wrote: > > > > Il giorno 2 set 2020, alle ore 17:17, Jan Kara <jack@suse.cz> ha scritto: > > > > On Wed 26-08-20 15:54:19, Jan Kara wrote: > >> On Mon 27-07-20 09:35:15, Jan Kara wrote: > >>> On Wed 22-07-20 11:13:28, Paolo Valente wrote: > >>>>> a) I don't think adding these samples to statistics helps in any way (you > >>>>> cannot improve the prediction power of the statistics by including in it > >>>>> some samples that are not directly related to the thing you try to > >>>>> predict). And think time is used to predict the answer to the question: If > >>>>> bfq queue becomes idle, how long will it take for new request to arrive? So > >>>>> second and further requests are simply irrelevant. > >>>>> > >>>> > >>>> Yes, you are super right in theory. > >>>> > >>>> Unfortunately this may not mean that your patch will do only good, for > >>>> the concerns in my previous email. > >>>> > >>>> So, here is a proposal to move forward: > >>>> 1) I test your patch on my typical set of > >>>> latency/guaranteed-bandwidth/total-throughput benchmarks > >>>> 2) You test your patch on a significant set of benchmarks in mmtests > >>>> > >>>> What do you think? > >>> > >>> Sure, I will queue runs for the patches with mmtests :). > >> > >> Sorry it took so long but I've hit a couple of technical snags when running > >> these tests (plus I was on vacation). So I've run the tests on 4 machines. > >> 2 with rotational disks with NCQ, 2 with SATA SSD. Results are mostly > >> neutral, details are below. > >> > >> For dbench, it seems to be generally neutral but the patches do fix > >> occasional weird outlier which are IMO caused exactly by bugs in the > >> heuristics I'm fixing. Things like (see the outlier for 4 clients > >> with vanilla kernel): > >> > >> vanilla bfq-waker-fixes > >> Amean 1 32.57 ( 0.00%) 32.10 ( 1.46%) > >> Amean 2 34.73 ( 0.00%) 34.68 ( 0.15%) > >> Amean 4 199.74 ( 0.00%) 45.76 ( 77.09%) > >> Amean 8 65.41 ( 0.00%) 65.47 ( -0.10%) > >> Amean 16 95.46 ( 0.00%) 96.61 ( -1.21%) > >> Amean 32 148.07 ( 0.00%) 147.66 ( 0.27%) > >> Amean 64 291.17 ( 0.00%) 289.44 ( 0.59%) > >> > >> For pgbench and bonnie, patches are neutral for all the machines. > >> > >> For reaim disk workload, patches are mostly neutral, just on one machine > >> with SSD they seem to improve XFS results and worsen ext4 results. But > >> results look rather noisy on that machine so it may be just a noise... > >> > >> For parallel dd(1) processes reading from multiple files, results are also > >> neutral all machines. > >> > >> For parallel dd(1) processes reading from a common file, results are also > >> neutral except for one machine with SSD on XFS (ext4 was fine) where there > >> seems to be consistent regression for 4 and more processes: > >> > >> vanilla bfq-waker-fixes > >> Amean 1 393.30 ( 0.00%) 391.02 ( 0.58%) > >> Amean 4 443.88 ( 0.00%) 517.16 ( -16.51%) > >> Amean 7 599.60 ( 0.00%) 748.68 ( -24.86%) > >> Amean 12 1134.26 ( 0.00%) 1255.62 ( -10.70%) > >> Amean 21 1940.50 ( 0.00%) 2206.29 ( -13.70%) > >> Amean 30 2381.08 ( 0.00%) 2735.69 ( -14.89%) > >> Amean 48 2754.36 ( 0.00%) 3258.93 ( -18.32%) > >> > >> I'll try to reproduce this regression and check what's happening... > >> > >> So what do you think, are you fine with merging my patches now? > > > > Paolo, any results from running your tests for these patches? I'd like to > > get these mostly obvious things merged so that we can move on... > > > > Hi, > sorry again for my delay. Tested this too, at last. No regression. So gladly > > Acked-by: Paolo Valente <paolo.valente@linaro.org> > > And thank you very much for your contributions and patience, Thanks. I'll add you ack and send all the patches to Jens. Honza
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index c66c3eaa9e26..4b1c9c5f57b6 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5192,8 +5192,16 @@ static void bfq_update_io_thinktime(struct bfq_data *bfqd, struct bfq_queue *bfqq) { struct bfq_ttime *ttime = &bfqq->ttime; - u64 elapsed = ktime_get_ns() - bfqq->ttime.last_end_request; - + u64 elapsed; + + /* + * We are really interested in how long it takes for the queue to + * become busy when there is no outstanding IO for this queue. So + * ignore cases when the bfq queue has already IO queued. + */ + if (bfqq->dispatched || bfq_bfqq_busy(bfqq)) + return; + elapsed = ktime_get_ns() - bfqq->ttime.last_end_request; elapsed = min_t(u64, elapsed, 2ULL * bfqd->bfq_slice_idle); ttime->ttime_samples = (7*ttime->ttime_samples + 256) / 8;
Currently whenever bfq queue has a request queued we add now - last_completion_time to the think time statistics. This is however misleading in case the process is able to submit several requests in parallel because e.g. if the queue has request completed at time T0 and then queues new requests at times T1, T2, then we will add T1-T0 and T2-T0 to think time statistics which just doesn't make any sence (the queue's think time is penalized by the queue being able to submit more IO). So add to think time statistics only time intervals when the queue had no IO pending. Signed-off-by: Jan Kara <jack@suse.cz> --- block/bfq-iosched.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)