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

[Bart Van Assche <bvanassche@acm.org>]

On 5/22/24 02:01, 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"}
> +
>   #ifdef CONFIG_BUFFER_HEAD
>   DECLARE_EVENT_CLASS(block_buffer,
>   
> @@ -82,6 +90,8 @@ TRACE_EVENT(block_rq_requeue,
>   		__field(  dev_t,	dev			)
>   		__field(  sector_t,	sector			)
>   		__field(  unsigned int,	nr_sector		)
> +		__field(  unsigned int,	ioprio_class		)
> +		__field(  unsigned int, ioprio_value		)
>   		__array(  char,		rwbs,	RWBS_LEN	)
>   		__dynamic_array( char,	cmd,	1		)
>   	),
> @@ -90,16 +100,19 @@ 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_class = rq->ioprio >> IOPRIO_CLASS_SHIFT & 0x3;
> +		__entry->ioprio_value = rq->ioprio & 0xff;

Why to split the I/O priority field when storing it in __entry instead of when
the values are printed? Combined the ioprio bitfields occupy 16 bits. The above
patch reserves 64 bits in __entry. I think that's overkill. Additionally, some
bits of the I/O priority bits are discarded by the above code before I/O
priority information is reported.

Please split the I/O priority information into the three fields defined in
include/uapi/linux/ioprio.h (class, hint, prio) and use the macros from that
header file for splitting I/O priority information.

Thanks,

Bart.