[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aceaeb4f-40ed-444e-84a7-39ca640fe913@nvidia.com>
Date: Fri, 19 Sep 2025 08:49:24 +0000
From: Chaitanya Kulkarni <chaitanyak@...dia.com>
To: Johannes Thumshirn <johannes.thumshirn@....com>, Jens Axboe
<axboe@...nel.dk>
CC: Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
<mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
"linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-trace-kernel@...r.kernel.org" <linux-trace-kernel@...r.kernel.org>,
"linux-btrace@...r.kernel.org" <linux-btrace@...r.kernel.org>, John Garry
<john.g.garry@...cle.com>, Hannes Reinecke <hare@...e.de>, Damien Le Moal
<dlemoal@...nel.org>, Christoph Hellwig <hch@....de>, Naohiro Aota
<naohiro.aota@....com>, Shinichiro Kawasaki <shinichiro.kawasaki@....com>,
"Martin K . Petersen" <martin.petersen@...cle.com>
Subject: Re: [PATCH 03/21] call BLKTRACESETUP2 ioctl per default to setup a
trace
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 <johannes.thumshirn@....com>
---
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
Powered by blists - more mailing lists