On 10/20/25 22:41, Johannes Thumshirn wrote:
> Add block trace commands for zone operations. These commands can only be
> handled with version 2 of the blktrace protocol. For version 1, warn if a
> command that does not fit into the 16 bits reserved for the command in
> this version is passed in.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
> include/uapi/linux/blktrace_api.h | 13 +++++++++++--
> kernel/trace/blktrace.c | 28 ++++++++++++++++++++++++----
> 2 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/blktrace_api.h b/include/uapi/linux/blktrace_api.h
> index 3a771b9802aa..925f78af939e 100644
> --- a/include/uapi/linux/blktrace_api.h
> +++ b/include/uapi/linux/blktrace_api.h
> @@ -26,11 +26,20 @@ 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 = 1ull << 16, /* zone append */
> + BLK_TC_ZONE_RESET = 1ull << 17, /* zone reset */
> + BLK_TC_ZONE_RESET_ALL = 1ull << 18, /* zone reset all */
> + BLK_TC_ZONE_FINISH = 1ull << 19, /* zone finish */
> + BLK_TC_ZONE_OPEN = 1ull << 20, /* zone open */
> + BLK_TC_ZONE_CLOSE = 1ull << 21, /* zone close */
> +
> + BLK_TC_END_V2 = 1ull << 21,
> };
>
> #define BLK_TC_SHIFT (16)
> -#define BLK_TC_ACT(act) ((act) << BLK_TC_SHIFT)
> +#define BLK_TC_ACT(act) ((u64)(act) << BLK_TC_SHIFT)
>
> /*
> * Basic trace actions
> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 8ffb218e9fb7..e8effb6cb393 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -163,8 +163,8 @@ static void relay_blktrace_event(struct blk_trace *bt, unsigned long sequence,
> bytes, what, error, cgid, cgid_len,
> pdu_data, pdu_len);
> return relay_blktrace_event1(bt, sequence, pid, cpu, sector, bytes,
> - lower_32_bits(what), error, cgid, cgid_len,
> - pdu_data, pdu_len);
> + what, error, cgid, cgid_len, pdu_data,
> + pdu_len);
> }
>
> /*
> @@ -342,10 +342,31 @@ 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_ACT(BLK_TC_ZONE_APPEND);
> + break;
> + case REQ_OP_ZONE_RESET:
> + what |= BLK_TC_ACT(BLK_TC_ZONE_RESET);
> + break;
> + case REQ_OP_ZONE_RESET_ALL:
> + what |= BLK_TC_ACT(BLK_TC_ZONE_RESET_ALL);
> + break;
> + case REQ_OP_ZONE_FINISH:
> + what |= BLK_TC_ACT(BLK_TC_ZONE_FINISH);
> + break;
> + case REQ_OP_ZONE_OPEN:
> + what |= BLK_TC_ACT(BLK_TC_ZONE_OPEN);
> + break;
> + case REQ_OP_ZONE_CLOSE:
> + what |= BLK_TC_ACT(BLK_TC_ZONE_CLOSE);
> + break;
> default:
> break;
> }
>
> + WARN_ON_ONCE(bt->version == 1 &&
> + (what >> BLK_TC_SHIFT) > BLK_TC_END_V1);
Shouldn't this be a "if (WARN_ON_ONCE())" and return doing nothing if true ?
> +
> if (cgid)
> what |= __BLK_TA_CGROUP;
>
> @@ -386,8 +407,7 @@ static void __blk_add_trace(struct blk_trace *bt, sector_t sector, int bytes,
> sequence = per_cpu_ptr(bt->sequence, cpu);
> (*sequence)++;
> relay_blktrace_event(bt, *sequence, pid, cpu, sector, bytes,
> - lower_32_bits(what), error, cgid, cgid_len,
> - pdu_data, pdu_len);
> + what, error, cgid, cgid_len, pdu_data, pdu_len);
> local_irq_restore(flags);
> }
>
--
Damien Le Moal
Western Digital Research