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 <[email protected]>
---
Changes in v4:
- Use macros to split ioprio.
- Print ioprio hint.
- Only storage ioprio in __entry.
---
---
include/trace/events/block.h | 77 +++++++++++++++++++++++-------------
1 file changed, 50 insertions(+), 27 deletions(-)
diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 0e128ad51460..209d54dc9dce 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,
@@ -79,27 +87,32 @@ TRACE_EVENT(block_rq_requeue,
TP_ARGS(rq),
TP_STRUCT__entry(
- __field( dev_t, dev )
- __field( sector_t, sector )
- __field( unsigned int, nr_sector )
- __array( char, rwbs, RWBS_LEN )
- __dynamic_array( char, cmd, 1 )
+ __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 )
),
TP_fast_assign(
__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,
@@ -109,12 +122,13 @@ DECLARE_EVENT_CLASS(block_rq_completion,
TP_ARGS(rq, error, nr_bytes),
TP_STRUCT__entry(
- __field( dev_t, dev )
- __field( sector_t, sector )
- __field( unsigned int, nr_sector )
- __field( int , error )
- __array( char, rwbs, RWBS_LEN )
- __dynamic_array( char, cmd, 1 )
+ __field( dev_t, dev )
+ __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 )
),
TP_fast_assign(
@@ -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)
);
/**
@@ -176,13 +194,14 @@ DECLARE_EVENT_CLASS(block_rq,
TP_ARGS(rq),
TP_STRUCT__entry(
- __field( dev_t, dev )
- __field( sector_t, sector )
- __field( unsigned int, nr_sector )
- __field( unsigned int, bytes )
- __array( char, rwbs, RWBS_LEN )
- __array( char, comm, TASK_COMM_LEN )
- __dynamic_array( char, cmd, 1 )
+ __field( dev_t, dev )
+ __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 )
),
TP_fast_assign(
@@ -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
On 6/11/24 12:35 AM, Dongliang Cui wrote:
> +#define IOPRIO_CLASS_STRINGS \
> + { IOPRIO_CLASS_NONE, "none" }, \
> + { IOPRIO_CLASS_RT, "rt" }, \
> + { IOPRIO_CLASS_BE, "be" }, \
> + { IOPRIO_CLASS_IDLE, "idle" }, \
> + { IOPRIO_CLASS_INVALID, "invalid"}
Shouldn't this array be defined in a C file instead of in a header file?
> @@ -79,27 +87,32 @@ TRACE_EVENT(block_rq_requeue,
> TP_ARGS(rq),
>
> TP_STRUCT__entry(
> - __field( dev_t, dev )
> - __field( sector_t, sector )
> - __field( unsigned int, nr_sector )
> - __array( char, rwbs, RWBS_LEN )
> - __dynamic_array( char, cmd, 1 )
> + __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 )
> ),
I see unnecessary whitespace changes. These changes make this patch harder to
read than necessary. Please undo the whitespace changes.
> DECLARE_EVENT_CLASS(block_rq_completion,
> @@ -109,12 +122,13 @@ DECLARE_EVENT_CLASS(block_rq_completion,
> TP_ARGS(rq, error, nr_bytes),
>
> TP_STRUCT__entry(
> - __field( dev_t, dev )
> - __field( sector_t, sector )
> - __field( unsigned int, nr_sector )
> - __field( int , error )
> - __array( char, rwbs, RWBS_LEN )
> - __dynamic_array( char, cmd, 1 )
> + __field( dev_t, dev )
> + __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 )
> ),
Also here, please do not reformat lines that are not modified otherwise.
> @@ -176,13 +194,14 @@ DECLARE_EVENT_CLASS(block_rq,
> TP_ARGS(rq),
>
> TP_STRUCT__entry(
> - __field( dev_t, dev )
> - __field( sector_t, sector )
> - __field( unsigned int, nr_sector )
> - __field( unsigned int, bytes )
> - __array( char, rwbs, RWBS_LEN )
> - __array( char, comm, TASK_COMM_LEN )
> - __dynamic_array( char, cmd, 1 )
> + __field( dev_t, dev )
> + __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 )
> ),
Same comment here.
Thanks,
Bart.
On Tue, 11 Jun 2024 09:26:54 -0700
Bart Van Assche <[email protected]> wrote:
> On 6/11/24 12:35 AM, Dongliang Cui wrote:
> > +#define IOPRIO_CLASS_STRINGS \
> > + { IOPRIO_CLASS_NONE, "none" }, \
> > + { IOPRIO_CLASS_RT, "rt" }, \
> > + { IOPRIO_CLASS_BE, "be" }, \
> > + { IOPRIO_CLASS_IDLE, "idle" }, \
> > + { IOPRIO_CLASS_INVALID, "invalid"}
>
> Shouldn't this array be defined in a C file instead of in a header file?
The way the TRACE_EVENT() macro works, this will not work in a C file.
> - 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)
> );
>
It's used for __print_symbolic() which the TRACE_EVENT() macro logic (using
header files) will expand it to something useful.
-- Steve
On 6/11/24 9:54 AM, Steven Rostedt wrote:
> On Tue, 11 Jun 2024 09:26:54 -0700
> Bart Van Assche <[email protected]> wrote:
>
>> On 6/11/24 12:35 AM, Dongliang Cui wrote:
>>> +#define IOPRIO_CLASS_STRINGS \
>>> + { IOPRIO_CLASS_NONE, "none" }, \
>>> + { IOPRIO_CLASS_RT, "rt" }, \
>>> + { IOPRIO_CLASS_BE, "be" }, \
>>> + { IOPRIO_CLASS_IDLE, "idle" }, \
>>> + { IOPRIO_CLASS_INVALID, "invalid"}
>>
>> Shouldn't this array be defined in a C file instead of in a header file?
>
> The way the TRACE_EVENT() macro works, this will not work in a C file.
Hmm ... if the above array is terminated with a { -1, NULL } sentinel and if
__print_symbolic() is changed into trace_print_symbols_seq(p, ...) then the above
array can be moved into a C file, isn't it?
Thanks,
Bart.
On Tue, 11 Jun 2024 10:09:12 -0700
Bart Van Assche <[email protected]> wrote:
> On 6/11/24 9:54 AM, Steven Rostedt wrote:
> > On Tue, 11 Jun 2024 09:26:54 -0700
> > Bart Van Assche <[email protected]> wrote:
> >
> >> On 6/11/24 12:35 AM, Dongliang Cui wrote:
> >>> +#define IOPRIO_CLASS_STRINGS \
> >>> + { IOPRIO_CLASS_NONE, "none" }, \
> >>> + { IOPRIO_CLASS_RT, "rt" }, \
> >>> + { IOPRIO_CLASS_BE, "be" }, \
> >>> + { IOPRIO_CLASS_IDLE, "idle" }, \
> >>> + { IOPRIO_CLASS_INVALID, "invalid"}
> >>
> >> Shouldn't this array be defined in a C file instead of in a header file?
> >
> > The way the TRACE_EVENT() macro works, this will not work in a C file.
>
> Hmm ... if the above array is terminated with a { -1, NULL } sentinel and if
> __print_symbolic() is changed into trace_print_symbols_seq(p, ...) then the above
> array can be moved into a C file, isn't it?
>
Then it breaks user space parsing. The reason for __print_symbolic() is
that libtraceevent knows how to parse it. If you put the array into a C
file, the above mappings will not show up in the tracefs format file for
the event, and you'll just get "[FAILED TO PARSE]" output from the user
space tracing tooling.
-- Steve
On Tue, 11 Jun 2024 13:17:37 -0400
Steven Rostedt <[email protected]> wrote:
> > Hmm ... if the above array is terminated with a { -1, NULL } sentinel and if
> > __print_symbolic() is changed into trace_print_symbols_seq(p, ...) then the above
> > array can be moved into a C file, isn't it?
> >
>
> Then it breaks user space parsing. The reason for __print_symbolic() is
> that libtraceevent knows how to parse it. If you put the array into a C
> file, the above mappings will not show up in the tracefs format file for
> the event, and you'll just get "[FAILED TO PARSE]" output from the user
> space tracing tooling.
Note, the trace headers are not normal headers. They are included multiple
times (when TRACE_HEADER_MULTI_READ is defined). Only one C file will
include this header with CREATE_TRACE_POINTS defined and these headers will
then build global C functions and variables.
So technically, this "array" is in C file and not in a header, as it will
not be created unless a C file includes it with CREATE_TRACE_POINTS, and
only one C file may do that (otherwise the kernel will fail to build).
-- Steve