linux-kernel - Re: [PATCH v3] block: Add ioprio to block

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b2e4a072-a339-444e-abea-29388dcbfee6@acm.org>
Date: Wed, 22 May 2024 09:51:16 -0700
From: Bart Van Assche <bvanassche@....org>
To: Dongliang Cui <dongliang.cui@...soc.com>, axboe@...nel.dk,
 rostedt@...dmis.org, mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
 ebiggers@...nel.org
Cc: ke.wang@...soc.com, hongyu.jin.cn@...il.com, niuzhiguo84@...il.com,
 hao_hao.wang@...soc.com, linux-block@...r.kernel.org,
 linux-kernel@...r.kernel.org, akailash@...gle.com,
 cuidongliang390@...il.com, Damien Le Moal <dlemoal@...nel.org>
Subject: Re: [PATCH v3] block: Add ioprio to block_rq tracepoint

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.