[v2] block: add blktrace support for zoned block device commands

[PATCH v2 11/15] blktrace: add block trace commands for zone operations

Posted by Johannes Thumshirn 4 months, 2 weeks ago

Add block trace commands for zone operations. These are added as a
separate set of 'block trace commands' shifted by 32bit so that they do
not interfere with the old 16bit wide trace command field in 'struct
blk_io_trace' action.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 include/uapi/linux/blktrace_api.h | 13 ++++++++++++-
 kernel/trace/blktrace.c           | 18 ++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/blktrace_api.h b/include/uapi/linux/blktrace_api.h
index d58ef484de49..0f336140ce4e 100644
--- a/include/uapi/linux/blktrace_api.h
+++ b/include/uapi/linux/blktrace_api.h
@@ -26,11 +26,22 @@ enum blktrace_cat {
 	BLK_TC_DRV_DATA	= 1 << 14,	/* binary per-driver data */
 	BLK_TC_FUA	= 1 << 15,	/* fua requests */
 
-	BLK_TC_END	= 1 << 15,	/* we've run out of bits! */
+	BLK_TC_END_V1	= 1 << 15,	/* we've run out of bits! */
+
+	BLK_TC_ZONE_APPEND	= 1 << 16ull,  	/* zone append */
+	BLK_TC_ZONE_RESET	= 1 << 17ull,	/* zone reset */
+	BLK_TC_ZONE_RESET_ALL	= 1 << 18ull,	/* zone reset all */
+	BLK_TC_ZONE_FINISH	= 1 << 19ull,	/* zone finish */
+	BLK_TC_ZONE_OPEN	= 1 << 20ull,	/* zone open */
+	BLK_TC_ZONE_CLOSE	= 1 << 21ull,	/* zone close */
+
+	BLK_TC_END_V2		= 1 << 21ull,
 };
 
 #define BLK_TC_SHIFT		(16)
 #define BLK_TC_ACT(act)		((act) << BLK_TC_SHIFT)
+#define BLK_TC_SHIFT2		(32)
+#define BLK_TC_ACT2(act)	((u64)(act) << BLK_TC_SHIFT2)
 
 /*
  * Basic trace actions
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 82ad626d6202..62f6cfcee4f6 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -333,6 +333,24 @@ static void __blk_add_trace(struct blk_trace *bt, sector_t sector, int bytes,
 	case REQ_OP_FLUSH:
 		what |= BLK_TC_ACT(BLK_TC_FLUSH);
 		break;
+	case REQ_OP_ZONE_APPEND:
+		what |= BLK_TC_ACT2(BLK_TC_ZONE_APPEND);
+		break;
+	case REQ_OP_ZONE_RESET:
+		what |= BLK_TC_ACT2(BLK_TC_ZONE_RESET);
+		break;
+	case REQ_OP_ZONE_RESET_ALL:
+		what |= BLK_TC_ACT2(BLK_TC_ZONE_RESET_ALL);
+		break;
+	case REQ_OP_ZONE_FINISH:
+		what |= BLK_TC_ACT2(BLK_TC_ZONE_FINISH);
+		break;
+	case REQ_OP_ZONE_OPEN:
+		what |= BLK_TC_ACT2(BLK_TC_ZONE_OPEN);
+		break;
+	case REQ_OP_ZONE_CLOSE:
+		what |= BLK_TC_ACT2(BLK_TC_ZONE_CLOSE);
+		break;
 	default:
 		break;
 	}
-- 
2.51.0

Re: [PATCH v2 11/15] blktrace: add block trace commands for zone operations

Posted by Christoph Hellwig 4 months, 1 week ago

On Thu, Sep 25, 2025 at 05:02:27PM +0200, Johannes Thumshirn wrote:
> Add block trace commands for zone operations. These are added as a
> separate set of 'block trace commands' shifted by 32bit so that they do
> not interfere with the old 16bit wide trace command field in 'struct
> blk_io_trace' action.

Can you explain how the commands are handled for old/new here?

Because I'd still much prefer to sort things out so that they make
sense for the new code if possible.  i.e. have a 32-bit command
and 32 bit flags, and use sensible encoding for the new one, and
remap the supported once to the old organically grown one.

Re: [PATCH v2 11/15] blktrace: add block trace commands for zone operations

Posted by Johannes Thumshirn 4 months ago

On 10/3/25 9:33 AM, Christoph Hellwig wrote:
> On Thu, Sep 25, 2025 at 05:02:27PM +0200, Johannes Thumshirn wrote:
>> Add block trace commands for zone operations. These are added as a
>> separate set of 'block trace commands' shifted by 32bit so that they do
>> not interfere with the old 16bit wide trace command field in 'struct
>> blk_io_trace' action.
> Can you explain how the commands are handled for old/new here?
>
> Because I'd still much prefer to sort things out so that they make
> sense for the new code if possible.  i.e. have a 32-bit command
> and 32 bit flags, and use sensible encoding for the new one, and
> remap the supported once to the old organically grown one.

Sure for the old commands everything is still in the lower 32bits, this 
has the nice property that we don't need to duplicate all the code for 
v1 and v2.

The commands added afterwards are intended to be in the upper 32bits, 
which are discarded if the user requests the v1 format.

At least this was the original plan. I think I badly messed up v2 as the 
new commands should re-start at 0 and be shifted up by 32bits.

Re: [PATCH v2 11/15] blktrace: add block trace commands for zone operations

Posted by hch 4 months ago

On Tue, Oct 07, 2025 at 01:08:00PM +0000, Johannes Thumshirn wrote:
> Sure for the old commands everything is still in the lower 32bits, this 
> has the nice property that we don't need to duplicate all the code for 
> v1 and v2.

I don't think you need to duplicate anything, just have a little
function that maps from the free-form v2 commands and flags to the
v1 field.  Preferably including a mapping of all unsupported ones to
a catchall unsupported command and flag each to indicate that the
trace includes something only visible with v2.

Re: [PATCH v2 11/15] blktrace: add block trace commands for zone operations

Posted by Johannes Thumshirn 4 months ago

On 10/8/25 8:14 AM, hch wrote:
> On Tue, Oct 07, 2025 at 01:08:00PM +0000, Johannes Thumshirn wrote:
>> Sure for the old commands everything is still in the lower 32bits, this
>> has the nice property that we don't need to duplicate all the code for
>> v1 and v2.
> I don't think you need to duplicate anything, just have a little
> function that maps from the free-form v2 commands and flags to the
> v1 field.  Preferably including a mapping of all unsupported ones to
> a catchall unsupported command and flag each to indicate that the
> trace includes something only visible with v2.

So I've tried making a translation function (which is the trivial part) 
but then it's a game of whack-a-mole to unbreak compilation, ftrace, etc..

I think it's not really worth the effort.

Re: [PATCH v2 11/15] blktrace: add block trace commands for zone operations

Posted by hch 4 months ago

On Thu, Oct 09, 2025 at 11:17:21AM +0000, Johannes Thumshirn wrote:
> On 10/8/25 8:14 AM, hch wrote:
> > On Tue, Oct 07, 2025 at 01:08:00PM +0000, Johannes Thumshirn wrote:
> >> Sure for the old commands everything is still in the lower 32bits, this
> >> has the nice property that we don't need to duplicate all the code for
> >> v1 and v2.
> > I don't think you need to duplicate anything, just have a little
> > function that maps from the free-form v2 commands and flags to the
> > v1 field.  Preferably including a mapping of all unsupported ones to
> > a catchall unsupported command and flag each to indicate that the
> > trace includes something only visible with v2.
> 
> So I've tried making a translation function (which is the trivial part) 
> but then it's a game of whack-a-mole to unbreak compilation, ftrace, etc..

What's the problem?

> I think it's not really worth the effort.

Why?  We really want a clean slate going forward.  Creating a permanent
split into legacy vs new commands seems very unfortunate.

Re: [PATCH v2 11/15] blktrace: add block trace commands for zone operations

Posted by Johannes Thumshirn 4 months ago

On 10/8/25 8:14 AM, hch wrote:
> On Tue, Oct 07, 2025 at 01:08:00PM +0000, Johannes Thumshirn wrote:
>> Sure for the old commands everything is still in the lower 32bits, this
>> has the nice property that we don't need to duplicate all the code for
>> v1 and v2.
> I don't think you need to duplicate anything, just have a little
> function that maps from the free-form v2 commands and flags to the
> v1 field.  Preferably including a mapping of all unsupported ones to
> a catchall unsupported command and flag each to indicate that the
> trace includes something only visible with v2.

I see your point, let me cook something.

Re: [PATCH v2 11/15] blktrace: add block trace commands for zone operations

Posted by Damien Le Moal 4 months, 1 week ago

On 9/26/25 00:02, Johannes Thumshirn wrote:
> Add block trace commands for zone operations. These are added as a
> separate set of 'block trace commands' shifted by 32bit so that they do
> not interfere with the old 16bit wide trace command field in 'struct
> blk_io_trace' action.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  include/uapi/linux/blktrace_api.h | 13 ++++++++++++-
>  kernel/trace/blktrace.c           | 18 ++++++++++++++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/blktrace_api.h b/include/uapi/linux/blktrace_api.h
> index d58ef484de49..0f336140ce4e 100644
> --- a/include/uapi/linux/blktrace_api.h
> +++ b/include/uapi/linux/blktrace_api.h
> @@ -26,11 +26,22 @@ enum blktrace_cat {
>  	BLK_TC_DRV_DATA	= 1 << 14,	/* binary per-driver data */
>  	BLK_TC_FUA	= 1 << 15,	/* fua requests */
>  
> -	BLK_TC_END	= 1 << 15,	/* we've run out of bits! */
> +	BLK_TC_END_V1	= 1 << 15,	/* we've run out of bits! */
> +
> +	BLK_TC_ZONE_APPEND	= 1 << 16ull,  	/* zone append */
> +	BLK_TC_ZONE_RESET	= 1 << 17ull,	/* zone reset */
> +	BLK_TC_ZONE_RESET_ALL	= 1 << 18ull,	/* zone reset all */
> +	BLK_TC_ZONE_FINISH	= 1 << 19ull,	/* zone finish */
> +	BLK_TC_ZONE_OPEN	= 1 << 20ull,	/* zone open */
> +	BLK_TC_ZONE_CLOSE	= 1 << 21ull,	/* zone close */

Isn't it more common/correct to do "1ULL << 21" ?



-- 
Damien Le Moal
Western Digital Research