Trace zone management operations on block devices.

As tracing of zoned block commands needs the upper 32bit of the widened
64bit action, only add traces to blktrace if user-space has requested
version 2 of the blktrace protocol.

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

diff --git a/include/uapi/linux/blktrace_api.h b/include/uapi/linux/blktrace_api.h
index ddc9fedf4955..e4b6fbbc40ee 100644
--- a/include/uapi/linux/blktrace_api.h
+++ b/include/uapi/linux/blktrace_api.h
@@ -64,6 +64,7 @@ enum blktrace_act {
 	__BLK_TA_REMAP,			/* bio was remapped */
 	__BLK_TA_ABORT,			/* request aborted */
 	__BLK_TA_DRV_DATA,		/* driver-specific binary data */
+	__BLK_TA_ZONE_MGMT,		/* zone management command was issued */
 	__BLK_TA_CGROUP = 1 << 8,	/* from a cgroup*/
 };
 
@@ -101,6 +102,7 @@ enum blktrace_notify {
 
 #define BLK_TA_ZONE_APPEND	(__BLK_TA_COMPLETE |\
 				 BLK_TC_ACT2(BLK_TC_ZONE_APPEND))
+#define BLK_TA_ZONE_MGMT	__BLK_TA_ZONE_MGMT
 
 #define BLK_TN_PROCESS		(__BLK_TN_PROCESS | BLK_TC_ACT(BLK_TC_NOTIFY))
 #define BLK_TN_TIMESTAMP	(__BLK_TN_TIMESTAMP | BLK_TC_ACT(BLK_TC_NOTIFY))
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index fea6e63ee27c..13424efbb2f6 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -1046,6 +1046,22 @@ static void blk_add_trace_getrq(void *ignore, struct bio *bio)
 	blk_add_trace_bio(bio->bi_bdev->bd_disk->queue, bio, BLK_TA_GETRQ, 0);
 }
 
+static void blk_add_trace_blkdev_zone_mgmt(void *ignore, struct bio *bio,
+					   sector_t nr_sectors)
+{
+	struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+	struct blk_trace *bt;
+
+	rcu_read_lock();
+	bt = rcu_dereference(q->blk_trace);
+	if (unlikely(!bt) || bt->version < 2) {
+		rcu_read_unlock();
+		return;
+	}
+	rcu_read_unlock();
+	blk_add_trace_bio(q, bio, BLK_TA_ZONE_MGMT, 0);
+}
+
 static void blk_add_trace_plug(void *ignore, struct request_queue *q)
 {
 	struct blk_trace *bt;
@@ -1221,6 +1237,9 @@ static void blk_register_tracepoints(void)
 	ret = register_trace_blk_zone_append_update_request_bio(
 		blk_add_trace_zone_update_request, NULL);
 	WARN_ON(ret);
+	ret = register_trace_blkdev_zone_mgmt(blk_add_trace_blkdev_zone_mgmt,
+					      NULL);
+	WARN_ON(ret);
 	ret = register_trace_block_plug(blk_add_trace_plug, NULL);
 	WARN_ON(ret);
 	ret = register_trace_block_unplug(blk_add_trace_unplug, NULL);
@@ -1240,6 +1259,7 @@ static void blk_unregister_tracepoints(void)
 	unregister_trace_block_split(blk_add_trace_split, NULL);
 	unregister_trace_block_unplug(blk_add_trace_unplug, NULL);
 	unregister_trace_block_plug(blk_add_trace_plug, NULL);
+	unregister_trace_blkdev_zone_mgmt(blk_add_trace_blkdev_zone_mgmt, NULL);
 	unregister_trace_blk_zone_append_update_request_bio(
 		blk_add_trace_zone_update_request, NULL);
 	unregister_trace_block_getrq(blk_add_trace_getrq, NULL);
-- 
2.51.0

On 9/26/25 00:02, Johannes Thumshirn wrote:
> Trace zone management operations on block devices.
> 
> As tracing of zoned block commands needs the upper 32bit of the widened
> 64bit action, only add traces to blktrace if user-space has requested
> version 2 of the blktrace protocol.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

Note: Are the zone management command completion traced ? I do not see a patch
for that...


-- 
Damien Le Moal
Western Digital Research

On 10/1/25 9:30 AM, Damien Le Moal wrote:
> On 9/26/25 00:02, Johannes Thumshirn wrote:
>> Trace zone management operations on block devices.
>>
>> As tracing of zoned block commands needs the upper 32bit of the widened
>> 64bit action, only add traces to blktrace if user-space has requested
>> version 2 of the blktrace protocol.
>>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
>
> Note: Are the zone management command completion traced ? I do not see a patch
> for that...
>
>

I finally had a chance to look into zone management command tracing 
again, but the problem here is we're having this pattern:

int blkdev_zone_mgmt(struct block_device *bdev, enum req_op op,
                      sector_t sector, sector_t nr_sectors)
{

         /* [...] */

         trace_blkdev_zone_mgmt(bio, nr_sectors);
         ret = submit_bio_wait(bio);
         bio_put(bio);

         return ret;

}


I'm not sure if it makes sense to do completion tracing here. At least 
we cannot do it in the endio handler as usual.

One thing to get the error and the duration would be the following:

int blkdev_zone_mgmt(struct block_device *bdev, enum req_op op,
                      sector_t sector, sector_t nr_sectors)
{

         /* [...] */

         trace_blkdev_zone_mgmt(bio, nr_sectors);
         ret = submit_bio_wait(bio);

+     trace_blkdev_zone_mgmt_completion(bio, nr_sectors, bio->bi_error);
         bio_put(bio);

         return ret;

}

On 10/8/25 22:29, Johannes Thumshirn wrote:
> I'm not sure if it makes sense to do completion tracing here. At least 
> we cannot do it in the endio handler as usual.
> 
> One thing to get the error and the duration would be the following:
> 
> int blkdev_zone_mgmt(struct block_device *bdev, enum req_op op,
>                       sector_t sector, sector_t nr_sectors)
> {
> 
>          /* [...] */
> 
>          trace_blkdev_zone_mgmt(bio, nr_sectors);
>          ret = submit_bio_wait(bio);
> 
> +     trace_blkdev_zone_mgmt_completion(bio, nr_sectors, bio->bi_error);
>          bio_put(bio);

That does seem OK to me. Maybe try and see how it looks ?
Though the request alloc, insert, dispatch and completion for this BIO will
still be traced, right ? If these events show correctly that this is a zone
management command (and which one it is), then we should not need the above.


-- 
Damien Le Moal
Western Digital Research

On 10/9/25 12:41 AM, Damien Le Moal wrote:
> That does seem OK to me. Maybe try and see how it looks ?
> Though the request alloc, insert, dispatch and completion for this BIO will
> still be traced, right ? If these events show correctly that this is a zone
> management command (and which one it is), then we should not need the above.

Yes I /think/ this is enough.