blk-iocost: skip empty flush bio in iocost

Commit Message

Chengming Zhou July 20, 2023, 12:14 p.m. UTC

From: Chengming Zhou <[e-mail]>

The flush bio may have data, may have no data (empty flush), we couldn't
calculate cost for empty flush bio. So we'd better just skip it for now.

Another side effect is that empty flush bio's bio_end_sector() is 0, cause
iocg->cursor reset to 0, may break the cost calculation of other bios.

This isn't good enough, since flush bio still consume the device bandwidth,
but flush request is special, can be merged randomly in the flush state
machine, we don't know how to calculate cost for it for now.

Its completion time also has flaws, which may include the pre-flush or
post-flush completion time, but I don't know if we need to fix that and
how to fix it.

Signed-off-by: Chengming Zhou <[e-mail]>
---
 block/blk-iocost.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Tejun Heo July 20, 2023, 7:59 p.m. UTC | #1

On Thu, Jul 20, 2023 at 08:14:41PM +0800, [e-mail] wrote:
> From: Chengming Zhou <[e-mail]>
> 
> The flush bio may have data, may have no data (empty flush), we couldn't
> calculate cost for empty flush bio. So we'd better just skip it for now.
> 
> Another side effect is that empty flush bio's bio_end_sector() is 0, cause
> iocg->cursor reset to 0, may break the cost calculation of other bios.
> 
> This isn't good enough, since flush bio still consume the device bandwidth,
> but flush request is special, can be merged randomly in the flush state
> machine, we don't know how to calculate cost for it for now.
> 
> Its completion time also has flaws, which may include the pre-flush or
> post-flush completion time, but I don't know if we need to fix that and
> how to fix it.
> 
> Signed-off-by: Chengming Zhou <[e-mail]>

It's unlikely that this will change behaviors in an observable way in most
cases. That said, yeah, ignoring it in terms of cost calculation is likely
the better thing to do.

Acked-by: Tejun Heo <[e-mail]>

Thanks.

Jens Axboe July 20, 2023, 8:02 p.m. UTC | #2

On Thu, 20 Jul 2023 20:14:41 +0800, [e-mail] wrote:
> The flush bio may have data, may have no data (empty flush), we couldn't
> calculate cost for empty flush bio. So we'd better just skip it for now.
> 
> Another side effect is that empty flush bio's bio_end_sector() is 0, cause
> iocg->cursor reset to 0, may break the cost calculation of other bios.
> 
> This isn't good enough, since flush bio still consume the device bandwidth,
> but flush request is special, can be merged randomly in the flush state
> machine, we don't know how to calculate cost for it for now.
> 
> [...]

Applied, thanks!

[1/1] blk-iocost: skip empty flush bio in iocost
      commit: 013adcbef165c3eaf73e297b7482290593815ab8

Best regards,

Patch

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 6084a9519883..e735b3e9997c 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2516,6 +2516,10 @@  static void calc_vtime_cost_builtin(struct bio *bio, struct ioc_gq *iocg,
 	u64 seek_pages = 0;
 	u64 cost = 0;
 
+	/* Can't calculate cost for empty bio */
+	if (!bio->bi_iter.bi_size)
+		goto out;
+
 	switch (bio_op(bio)) {
 	case REQ_OP_READ:
 		coef_seqio	= ioc->params.lcoefs[LCOEF_RSEQIO];