[v5] block: Add ioprio to block_rq tracepoint

[PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by Dongliang Cui 1 year, 6 months ago

Sometimes we need to track the processing order of requests with
ioprio set. So the ioprio of request can be useful information.

Example：

block_rq_insert: 8,0 RA 16384 () 6500840 + 32 be,0,6 [binder:815_3]
block_rq_issue: 8,0 RA 16384 () 6500840 + 32 be,0,6 [binder:815_3]
block_rq_complete: 8,0 RA () 6500840 + 32 be,0,6 [0]

Signed-off-by: Dongliang Cui <dongliang.cui@unisoc.com>
---
Changes in v5:
 - Remove redundant changes.
---
---
 include/trace/events/block.h | 41 ++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 0e128ad51460..1527d5d45e01 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -9,9 +9,17 @@
 #include <linux/blkdev.h>
 #include <linux/buffer_head.h>
 #include <linux/tracepoint.h>
+#include <uapi/linux/ioprio.h>
 
 #define RWBS_LEN	8
 
+#define IOPRIO_CLASS_STRINGS \
+	{ IOPRIO_CLASS_NONE,	"none" }, \
+	{ IOPRIO_CLASS_RT,	"rt" }, \
+	{ IOPRIO_CLASS_BE,	"be" }, \
+	{ IOPRIO_CLASS_IDLE,	"idle" }, \
+	{ IOPRIO_CLASS_INVALID,	"invalid"}
+
 #ifdef CONFIG_BUFFER_HEAD
 DECLARE_EVENT_CLASS(block_buffer,
 
@@ -82,6 +90,7 @@ TRACE_EVENT(block_rq_requeue,
 		__field(  dev_t,	dev			)
 		__field(  sector_t,	sector			)
 		__field(  unsigned int,	nr_sector		)
+		__field(  unsigned short, ioprio		)
 		__array(  char,		rwbs,	RWBS_LEN	)
 		__dynamic_array( char,	cmd,	1		)
 	),
@@ -90,16 +99,20 @@ TRACE_EVENT(block_rq_requeue,
 		__entry->dev	   = rq->q->disk ? disk_devt(rq->q->disk) : 0;
 		__entry->sector    = blk_rq_trace_sector(rq);
 		__entry->nr_sector = blk_rq_trace_nr_sectors(rq);
+		__entry->ioprio    = rq->ioprio;
 
 		blk_fill_rwbs(__entry->rwbs, rq->cmd_flags);
 		__get_str(cmd)[0] = '\0';
 	),
 
-	TP_printk("%d,%d %s (%s) %llu + %u [%d]",
+	TP_printk("%d,%d %s (%s) %llu + %u %s,%u,%u [%d]",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->rwbs, __get_str(cmd),
-		  (unsigned long long)__entry->sector,
-		  __entry->nr_sector, 0)
+		  (unsigned long long)__entry->sector, __entry->nr_sector,
+		  __print_symbolic(IOPRIO_PRIO_CLASS(__entry->ioprio),
+				   IOPRIO_CLASS_STRINGS),
+		  IOPRIO_PRIO_HINT(__entry->ioprio),
+		  IOPRIO_PRIO_LEVEL(__entry->ioprio),  0)
 );
 
 DECLARE_EVENT_CLASS(block_rq_completion,
@@ -113,6 +126,7 @@ DECLARE_EVENT_CLASS(block_rq_completion,
 		__field(  sector_t,	sector			)
 		__field(  unsigned int,	nr_sector		)
 		__field(  int	,	error			)
+		__field(  unsigned short, ioprio		)
 		__array(  char,		rwbs,	RWBS_LEN	)
 		__dynamic_array( char,	cmd,	1		)
 	),
@@ -122,16 +136,20 @@ DECLARE_EVENT_CLASS(block_rq_completion,
 		__entry->sector    = blk_rq_pos(rq);
 		__entry->nr_sector = nr_bytes >> 9;
 		__entry->error     = blk_status_to_errno(error);
+		__entry->ioprio    = rq->ioprio;
 
 		blk_fill_rwbs(__entry->rwbs, rq->cmd_flags);
 		__get_str(cmd)[0] = '\0';
 	),
 
-	TP_printk("%d,%d %s (%s) %llu + %u [%d]",
+	TP_printk("%d,%d %s (%s) %llu + %u %s,%u,%u [%d]",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->rwbs, __get_str(cmd),
-		  (unsigned long long)__entry->sector,
-		  __entry->nr_sector, __entry->error)
+		  (unsigned long long)__entry->sector, __entry->nr_sector,
+		  __print_symbolic(IOPRIO_PRIO_CLASS(__entry->ioprio),
+				   IOPRIO_CLASS_STRINGS),
+		  IOPRIO_PRIO_HINT(__entry->ioprio),
+		  IOPRIO_PRIO_LEVEL(__entry->ioprio), __entry->error)
 );
 
 /**
@@ -180,6 +198,7 @@ DECLARE_EVENT_CLASS(block_rq,
 		__field(  sector_t,	sector			)
 		__field(  unsigned int,	nr_sector		)
 		__field(  unsigned int,	bytes			)
+		__field(  unsigned short, ioprio		)
 		__array(  char,		rwbs,	RWBS_LEN	)
 		__array(  char,         comm,   TASK_COMM_LEN   )
 		__dynamic_array( char,	cmd,	1		)
@@ -190,17 +209,21 @@ DECLARE_EVENT_CLASS(block_rq,
 		__entry->sector    = blk_rq_trace_sector(rq);
 		__entry->nr_sector = blk_rq_trace_nr_sectors(rq);
 		__entry->bytes     = blk_rq_bytes(rq);
+		__entry->ioprio	   = rq->ioprio;
 
 		blk_fill_rwbs(__entry->rwbs, rq->cmd_flags);
 		__get_str(cmd)[0] = '\0';
 		memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
 	),
 
-	TP_printk("%d,%d %s %u (%s) %llu + %u [%s]",
+	TP_printk("%d,%d %s %u (%s) %llu + %u %s,%u,%u [%s]",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->rwbs, __entry->bytes, __get_str(cmd),
-		  (unsigned long long)__entry->sector,
-		  __entry->nr_sector, __entry->comm)
+		  (unsigned long long)__entry->sector, __entry->nr_sector,
+		  __print_symbolic(IOPRIO_PRIO_CLASS(__entry->ioprio),
+				   IOPRIO_CLASS_STRINGS),
+		  IOPRIO_PRIO_HINT(__entry->ioprio),
+		  IOPRIO_PRIO_LEVEL(__entry->ioprio), __entry->comm)
 );
 
 /**
-- 
2.25.1

Re: [PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by Jens Axboe 1 year, 5 months ago

On Fri, 14 Jun 2024 15:49:36 +0800, Dongliang Cui wrote:
> Sometimes we need to track the processing order of requests with
> ioprio set. So the ioprio of request can be useful information.
> 
> Example：
> 
> block_rq_insert: 8,0 RA 16384 () 6500840 + 32 be,0,6 [binder:815_3]
> block_rq_issue: 8,0 RA 16384 () 6500840 + 32 be,0,6 [binder:815_3]
> block_rq_complete: 8,0 RA () 6500840 + 32 be,0,6 [0]
> 
> [...]

Applied, thanks!

[1/1] block: Add ioprio to block_rq tracepoint
      commit: aa6ff4eb7c10d9a6532db3ea9e78124bf14e70ae

Best regards,
-- 
Jens Axboe

Re: [PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by Chaitanya Kulkarni 1 year, 6 months ago

On 6/14/24 00:49, Dongliang Cui wrote:
> Sometimes we need to track the processing order of requests with
> ioprio set. So the ioprio of request can be useful information.
>
> Example：
>
> block_rq_insert: 8,0 RA 16384 () 6500840 + 32 be,0,6 [binder:815_3]
> block_rq_issue: 8,0 RA 16384 () 6500840 + 32 be,0,6 [binder:815_3]
> block_rq_complete: 8,0 RA () 6500840 + 32 be,0,6 [0]
>
> Signed-off-by: Dongliang Cui<dongliang.cui@unisoc.com>

Looks useful to me.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by dongliang cui 1 year, 5 months ago

On Tue, Jun 18, 2024 at 8:29 AM Chaitanya Kulkarni
<chaitanyak@nvidia.com> wrote:
>
> On 6/14/24 00:49, Dongliang Cui wrote:
> > Sometimes we need to track the processing order of requests with
> > ioprio set. So the ioprio of request can be useful information.
> >
> > Example：
> >
> > block_rq_insert: 8,0 RA 16384 () 6500840 + 32 be,0,6 [binder:815_3]
> > block_rq_issue: 8,0 RA 16384 () 6500840 + 32 be,0,6 [binder:815_3]
> > block_rq_complete: 8,0 RA () 6500840 + 32 be,0,6 [0]
> >
> > Signed-off-by: Dongliang Cui<dongliang.cui@unisoc.com>
>
> Looks useful to me.
>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
>
> -ck
>
>
kindly ping...

Re: [PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by Bart Van Assche 1 year, 6 months ago

On 6/14/24 12:49 AM, Dongliang Cui wrote:
> -	TP_printk("%d,%d %s (%s) %llu + %u [%d]",
> +	TP_printk("%d,%d %s (%s) %llu + %u %s,%u,%u [%d]",
>   		  MAJOR(__entry->dev), MINOR(__entry->dev),
>   		  __entry->rwbs, __get_str(cmd),
> -		  (unsigned long long)__entry->sector,
> -		  __entry->nr_sector, 0)
> +		  (unsigned long long)__entry->sector, __entry->nr_sector,
> +		  __print_symbolic(IOPRIO_PRIO_CLASS(__entry->ioprio),
> +				   IOPRIO_CLASS_STRINGS),
> +		  IOPRIO_PRIO_HINT(__entry->ioprio),
> +		  IOPRIO_PRIO_LEVEL(__entry->ioprio),  0)
>   );

Do we really want to include the constant "[0]" in the tracing output?

Otherwise this patch looks good to me.

Thanks,

Bart.

Re: [PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by dongliang cui 1 year, 6 months ago

On Sat, Jun 15, 2024 at 12:41 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 6/14/24 12:49 AM, Dongliang Cui wrote:
> > -     TP_printk("%d,%d %s (%s) %llu + %u [%d]",
> > +     TP_printk("%d,%d %s (%s) %llu + %u %s,%u,%u [%d]",
> >                 MAJOR(__entry->dev), MINOR(__entry->dev),
> >                 __entry->rwbs, __get_str(cmd),
> > -               (unsigned long long)__entry->sector,
> > -               __entry->nr_sector, 0)
> > +               (unsigned long long)__entry->sector, __entry->nr_sector,
> > +               __print_symbolic(IOPRIO_PRIO_CLASS(__entry->ioprio),
> > +                                IOPRIO_CLASS_STRINGS),
> > +               IOPRIO_PRIO_HINT(__entry->ioprio),
> > +               IOPRIO_PRIO_LEVEL(__entry->ioprio),  0)
> >   );
>
> Do we really want to include the constant "[0]" in the tracing output?
This is how it is printed in the source code.
From the code flow point of view, there is no need to print this value
in trace_block_rq_requeue.
Do we need to consider the issue of uniform printing format? If not, I
think we can delete it.
>
> Otherwise this patch looks good to me.
>
> Thanks,
>
> Bart.
>

Re: [PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by Bart Van Assche 1 year, 6 months ago

On 6/17/24 12:59 AM, dongliang cui wrote:
> On Sat, Jun 15, 2024 at 12:41 AM Bart Van Assche <bvanassche@acm.org> wrote:
>>
>> On 6/14/24 12:49 AM, Dongliang Cui wrote:
>>> -     TP_printk("%d,%d %s (%s) %llu + %u [%d]",
>>> +     TP_printk("%d,%d %s (%s) %llu + %u %s,%u,%u [%d]",
>>>                  MAJOR(__entry->dev), MINOR(__entry->dev),
>>>                  __entry->rwbs, __get_str(cmd),
>>> -               (unsigned long long)__entry->sector,
>>> -               __entry->nr_sector, 0)
>>> +               (unsigned long long)__entry->sector, __entry->nr_sector,
>>> +               __print_symbolic(IOPRIO_PRIO_CLASS(__entry->ioprio),
>>> +                                IOPRIO_CLASS_STRINGS),
>>> +               IOPRIO_PRIO_HINT(__entry->ioprio),
>>> +               IOPRIO_PRIO_LEVEL(__entry->ioprio),  0)
>>>    );
>>
>> Do we really want to include the constant "[0]" in the tracing output?
> This is how it is printed in the source code.
>  From the code flow point of view, there is no need to print this value
> in trace_block_rq_requeue.
> Do we need to consider the issue of uniform printing format? If not, I
> think we can delete it.

I'm not aware of any other tracing statement that prints out a constant.
Is there perhaps something that I'm missing or overlooking?

Thanks,

Bart.

Re: [PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by Steven Rostedt 1 year, 6 months ago

On Mon, 17 Jun 2024 10:02:48 -0700
Bart Van Assche <bvanassche@acm.org> wrote:

> >> Do we really want to include the constant "[0]" in the tracing output?  
> > This is how it is printed in the source code.
> >  From the code flow point of view, there is no need to print this value
> > in trace_block_rq_requeue.
> > Do we need to consider the issue of uniform printing format? If not, I
> > think we can delete it.  
> 
> I'm not aware of any other tracing statement that prints out a constant.
> Is there perhaps something that I'm missing or overlooking?

The only time that is done, is if the trace event is used in multiple
places and there's one place that the value will always be the same.

-- Steve

Re: [PATCH v5] block: Add ioprio to block_rq tracepoint

Posted by Bart Van Assche 1 year, 6 months ago

On 6/17/24 10:07 AM, Steven Rostedt wrote:
> On Mon, 17 Jun 2024 10:02:48 -0700
> Bart Van Assche <bvanassche@acm.org> wrote:
> 
>>>> Do we really want to include the constant "[0]" in the tracing output?
>>> This is how it is printed in the source code.
>>>   From the code flow point of view, there is no need to print this value
>>> in trace_block_rq_requeue.
>>> Do we need to consider the issue of uniform printing format? If not, I
>>> think we can delete it.
>>
>> I'm not aware of any other tracing statement that prints out a constant.
>> Is there perhaps something that I'm missing or overlooking?
> 
> The only time that is done, is if the trace event is used in multiple
> places and there's one place that the value will always be the same.

Thanks for the clarification Steven.

Hence:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>