On 7/13/26 02:36, Andrey Zhadchenko wrote:
> Currently all requests for request-based dm drivers are falling in
> the last bin for both normal and precise counting due to incorrect
> calculations.
> 
> Ordinary counting expects that start_time passed to dm_stats_account_io()
> is an absolute value and substracts it from jiffies to get request time.
> However rq_end_stats() calculates and passes request duration. So we
> actually get jiffies - <request_duration_in_jiffies>, which is incorrect.
> rq_end_stats() should not do the substraction.
> 
> For precise calulation stats->aux_duration_ns is used. It is calculated
> in dm_stats_account_io() like that:
> stats_aux->duration_ns = ktime_to_ns(ktime_get()) - stats_aux->duration_ns;
> The problem is that stats_aux->duration_ns() is set by
> dm_stats_record_start() which is called only for bio-based targets.
> It should also be called in dm_start_request().
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-103846
> Signed-off-by: Andrey Zhadchenko <[email protected]>
> ---
>  drivers/md/dm-rq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
> index 2177666c64f32..58ad4c94d6119 100644
> --- a/drivers/md/dm-rq.c
> +++ b/drivers/md/dm-rq.c
> @@ -129,7 +129,6 @@ static void rq_end_stats(struct mapped_device *md, struct 
> request *orig)
>       if (unlikely(dm_stats_used(&md->stats))) {
>               struct dm_rq_target_io *tio = tio_from_request(orig);
>  
> -             tio->duration_jiffies = jiffies - tio->duration_jiffies;

Having variable called duration but having start time in it is a mess, can we 
rename?

Plus this seems like mainstream bug.

Also in dm_start_request() probably the same problem, there jiffies-0 will be 
"duration".

>               dm_stats_account_io(&md->stats, rq_data_dir(orig),
>                                   blk_rq_pos(orig), tio->n_sectors, true,
>                                   tio->duration_jiffies, &tio->stats_aux);
> @@ -448,6 +447,8 @@ static void dm_start_request(struct mapped_device *md, 
> struct request *orig)
>  
>               tio->duration_jiffies = jiffies;
>               tio->n_sectors = blk_rq_sectors(orig);
> +
> +             dm_stats_record_start(&md->stats, &tio->stats_aux);



>               dm_stats_account_io(&md->stats, rq_data_dir(orig),
>                                   blk_rq_pos(orig), tio->n_sectors, false, 0,
>                                   &tio->stats_aux);

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to