[PATCH] block/bfq: Enable I/O statistics

[PATCH] block/bfq: Enable I/O statistics

[Bart Van Assche]

BFQ uses io_start_time_ns. That member variable is only set if I/O
statistics are enabled. Hence this patch that enables I/O statistics
at the time BFQ is associated with a request queue.

Compile-tested only.

Reported-by: Cixi Geng <cixi.geng1@unisoc.com>
Cc: Cixi Geng <cixi.geng1@unisoc.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Yu Kuai <yukuai3@huawei.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/bfq-iosched.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 0d46cb728bbf..519862d82473 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -7046,6 +7046,7 @@ static void bfq_exit_queue(struct elevator_queue *e)
 	spin_unlock_irq(&bfqd->lock);
 #endif
 
+	blk_stat_disable_accounting(bfqd->queue);
 	wbt_enable_default(bfqd->queue);
 
 	kfree(bfqd);
@@ -7189,6 +7190,8 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e)
 	bfq_init_entity(&bfqd->oom_bfqq.entity, bfqd->root_group);
 
 	wbt_disable_default(q);
+	blk_stat_enable_accounting(q);
+
 	return 0;
 
 out_free:

Re: [PATCH] block/bfq: Enable I/O statistics

[Jan Kara]

On Mon 13-06-22 09:32:34, Bart Van Assche wrote:
> BFQ uses io_start_time_ns. That member variable is only set if I/O
> statistics are enabled. Hence this patch that enables I/O statistics
> at the time BFQ is associated with a request queue.
> 
> Compile-tested only.
> 
> Reported-by: Cixi Geng <cixi.geng1@unisoc.com>
> Cc: Cixi Geng <cixi.geng1@unisoc.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Yu Kuai <yukuai3@huawei.com>
> Cc: Paolo Valente <paolo.valente@unimore.it>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Looks good. Thanks for the fix. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/bfq-iosched.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 0d46cb728bbf..519862d82473 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -7046,6 +7046,7 @@ static void bfq_exit_queue(struct elevator_queue *e)
>  	spin_unlock_irq(&bfqd->lock);
>  #endif
>  
> +	blk_stat_disable_accounting(bfqd->queue);
>  	wbt_enable_default(bfqd->queue);
>  
>  	kfree(bfqd);
> @@ -7189,6 +7190,8 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e)
>  	bfq_init_entity(&bfqd->oom_bfqq.entity, bfqd->root_group);
>  
>  	wbt_disable_default(q);
> +	blk_stat_enable_accounting(q);
> +
>  	return 0;
>  
>  out_free:
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] block/bfq: Enable I/O statistics

[Jens Axboe]

On 6/13/22 10:32 AM, Bart Van Assche wrote:
> BFQ uses io_start_time_ns. That member variable is only set if I/O
> statistics are enabled. Hence this patch that enables I/O statistics
> at the time BFQ is associated with a request queue.
> 
> Compile-tested only.

Have you runtime tested it now?

-- 
Jens Axboe

Re: [PATCH] block/bfq: Enable I/O statistics

[Bart Van Assche]

On 6/16/22 13:50, Jens Axboe wrote:
> On 6/13/22 10:32 AM, Bart Van Assche wrote:
>> BFQ uses io_start_time_ns. That member variable is only set if I/O
>> statistics are enabled. Hence this patch that enables I/O statistics
>> at the time BFQ is associated with a request queue.
>>
>> Compile-tested only.
> 
> Have you runtime tested it now?

This patch has been tested lightly: I ran blktests in a VM against a 
kernel that includes this patch. I'm not a BFQ expert so I was hoping 
for feedback from Paolo.

Thanks,

Bart.

Re: [PATCH] block/bfq: Enable I/O statistics

[Jens Axboe]

On 6/16/22 3:14 PM, Bart Van Assche wrote:
> On 6/16/22 13:50, Jens Axboe wrote:
>> On 6/13/22 10:32 AM, Bart Van Assche wrote:
>>> BFQ uses io_start_time_ns. That member variable is only set if I/O
>>> statistics are enabled. Hence this patch that enables I/O statistics
>>> at the time BFQ is associated with a request queue.
>>>
>>> Compile-tested only.
>>
>> Have you runtime tested it now?
> 
> This patch has been tested lightly: I ran blktests in a VM against a
> kernel that includes this patch. I'm not a BFQ expert so I was hoping
> for feedback from Paolo.

Sounds good - and Jan reviewed it, and I consider him the defactor BFQ
maintainer anyway these days :-)

-- 
Jens Axboe

Re: [PATCH] block/bfq: Enable I/O statistics

[Jens Axboe]

On Mon, 13 Jun 2022 09:32:34 -0700, Bart Van Assche wrote:
> BFQ uses io_start_time_ns. That member variable is only set if I/O
> statistics are enabled. Hence this patch that enables I/O statistics
> at the time BFQ is associated with a request queue.
> 
> Compile-tested only.
> 
> 
> [...]

Applied, thanks!

[1/1] block/bfq: Enable I/O statistics
      (no commit info)

Best regards,
-- 
Jens Axboe