Re: [PATCH 03/21] call BLKTRACESETUP2 ioctl per default to setup a trace
From: Chaitanya Kulkarni <hidden>
Date: 2025-09-19 08:49:27
Also in:
linux-block, lkml
Subsystem:
the rest · Maintainer:
Linus Torvalds
On 9/9/25 04:07, Johannes Thumshirn wrote: Call BLKTRACESETUP2 ioctl per default and if the kernel does not support this ioctl because it is too old, fall back to calling BLKTRACESETUP. Signed-off-by: Johannes Thumshirn <redacted> --- blktrace.c | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-)
diff --git a/blktrace.c b/blktrace.c
index 038b2cb..72562fd 100644
--- a/blktrace.c
+++ b/blktrace.c@@ -279,7 +279,7 @@ static int max_cpus; static int ncpus; static cpu_set_t *online_cpus; static int pagesize; -static int act_mask = ~0U; +static unsigned long long act_mask = ~0U;
~0ULL ? static int kill_running_trace; static int stop_watch; static int piped_output;
@@ -1067,6 +1067,36 @@ static void close_client_connections(void) } } +static int setup_buts2(void) +{ + struct list_head *p; + int ret = 0; + + __list_for_each(p, &devpaths) { + struct blk_user_trace_setup2 buts2; + struct devpath *dpp = list_entry(p, struct devpath, head); + + memset(&buts2, 0, sizeof(buts2)); + buts2.buf_size = buf_size; + buts2.buf_nr = buf_nr; + buts2.act_mask = act_mask; +
1. Original code (BLKTRACESETUP)
struct blk_user_trace_setup {
char name[32]; /* output */
__u16 act_mask; /* input */
...
};
static int act_mask = ~0U; /* global */
In handle_args():
int act_mask_tmp;
if (act_mask_tmp != 0)
act_mask = act_mask_tmp;
Later in setup_buts():
buts.act_mask = act_mask;
Here everything lines up:
handle_args() -> int act_mask_tmp
act_mask (global) -> int
buts.act_mask -> __u16
No truncation issues as long as the mask fits in 16 bits
(which it always did in the original design).
2. New code (BLKTRACESETUP2)
struct blk_user_trace_setup2 {
char name[32]; /* output */
__u64 act_mask; /* input */
...
};
static unsigned long long act_mask = ~0U; /* global changed */
In handle_args() (still unchanged):
int act_mask_tmp;
if (act_mask_tmp != 0)
act_mask = act_mask_tmp;
Later in setup_buts2():
buts2.act_mask = act_mask;
Here is the issue:
handle_args() still uses int act_mask_tmp.
global act_mask is now unsigned long long.
buts2.act_mask is __u64.
above truncation of bits can lead to compiler/tools warnings ?
also if in the future|act_mask| uses bits above 31, they could
never be set because the input path (|int act_mask_tmp|) can’t
represent them ?
-ck