[RFC] traceevent/block: Add REQ_ATOMIC flag to block trace events

[RFC] traceevent/block: Add REQ_ATOMIC flag to block trace events

[Ritesh Harjani (IBM)]

Filesystems like XFS can implement atomic write I/O using either REQ_ATOMIC
flag set in the bio or via CoW operation. It will be useful if we have a
flag in trace events to distinguish between the two. This patch adds
char 'a' to rwbs field of the trace events if REQ_ATOMIC flag is set in
the bio.

<W/ REQ_ATOMIC>
=================
xfs_io-1107    [002] .....   406.206441: block_rq_issue: 8,48 WSa 16384 () 768 + 32 none,0,0 [xfs_io]
<idle>-0       [002] ..s1.   406.209918: block_rq_complete: 8,48 WSa () 768 + 32 none,0,0 [0]

<W/O REQ_ATOMIC>
===============
xfs_io-1108    [002] .....   411.212317: block_rq_issue: 8,48 WS 16384 () 1024 + 32 none,0,0 [xfs_io]
<idle>-0       [002] ..s1.   411.215842: block_rq_complete: 8,48 WS () 1024 + 32 none,0,0 [0]

Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 include/trace/events/block.h | 2 +-
 kernel/trace/blktrace.c      | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index bd0ea07338eb..de538b110ea1 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -11,7 +11,7 @@
 #include <linux/tracepoint.h>
 #include <uapi/linux/ioprio.h>

-#define RWBS_LEN	8
+#define RWBS_LEN	9

 #define IOPRIO_CLASS_STRINGS \
 	{ IOPRIO_CLASS_NONE,	"none" }, \
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 3679a6d18934..6badf296ab2b 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -1896,6 +1896,8 @@ void blk_fill_rwbs(char *rwbs, blk_opf_t opf)
 		rwbs[i++] = 'S';
 	if (opf & REQ_META)
 		rwbs[i++] = 'M';
+	if (opf & REQ_ATOMIC)
+		rwbs[i++] = 'a';

 	rwbs[i] = '\0';
 }
--
2.48.1

Re: [RFC] traceevent/block: Add REQ_ATOMIC flag to block trace events

[John Garry]

On 03/04/2025 06:28, Ritesh Harjani (IBM) wrote:
> Filesystems like XFS can implement atomic write I/O using either REQ_ATOMIC
> flag set in the bio or via CoW operation. It will be useful if we have a
> flag in trace events to distinguish between the two. 

I suppose that this could be useful. So far I test with block driver 
traces, i.e. NVMe or SCSI internal traces, just to ensure that we see 
the requests sent as expected

This patch adds
> char 'a' to rwbs field of the trace events if REQ_ATOMIC flag is set in
> the bio.

All others use uppercase characters, so I suggest that you continue to 
use that. Since 'A' is already used, how about 'U' for untorn? Or 'T' 
for aTOMic :)

> 
> <W/ REQ_ATOMIC>
> =================
> xfs_io-1107    [002] .....   406.206441: block_rq_issue: 8,48 WSa 16384 () 768 + 32 none,0,0 [xfs_io]
> <idle>-0       [002] ..s1.   406.209918: block_rq_complete: 8,48 WSa () 768 + 32 none,0,0 [0]
> 
> <W/O REQ_ATOMIC>
> ===============
> xfs_io-1108    [002] .....   411.212317: block_rq_issue: 8,48 WS 16384 () 1024 + 32 none,0,0 [xfs_io]
> <idle>-0       [002] ..s1.   411.215842: block_rq_complete: 8,48 WS () 1024 + 32 none,0,0 [0]
> 
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> ---
>   include/trace/events/block.h | 2 +-
>   kernel/trace/blktrace.c      | 2 ++
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/trace/events/block.h b/include/trace/events/block.h
> index bd0ea07338eb..de538b110ea1 100644
> --- a/include/trace/events/block.h
> +++ b/include/trace/events/block.h
> @@ -11,7 +11,7 @@
>   #include <linux/tracepoint.h>
>   #include <uapi/linux/ioprio.h>
> 
> -#define RWBS_LEN	8
> +#define RWBS_LEN	9
> 
>   #define IOPRIO_CLASS_STRINGS \
>   	{ IOPRIO_CLASS_NONE,	"none" }, \
> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 3679a6d18934..6badf296ab2b 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -1896,6 +1896,8 @@ void blk_fill_rwbs(char *rwbs, blk_opf_t opf)
>   		rwbs[i++] = 'S';
>   	if (opf & REQ_META)
>   		rwbs[i++] = 'M';
> +	if (opf & REQ_ATOMIC)
> +		rwbs[i++] = 'a';
> 
>   	rwbs[i] = '\0';
>   }
> --
> 2.48.1
> 

Re: [RFC] traceevent/block: Add REQ_ATOMIC flag to block trace events

[Ritesh Harjani]

John Garry <john.g.garry@oracle.com> writes:

> On 03/04/2025 06:28, Ritesh Harjani (IBM) wrote:
>> Filesystems like XFS can implement atomic write I/O using either REQ_ATOMIC
>> flag set in the bio or via CoW operation. It will be useful if we have a
>> flag in trace events to distinguish between the two. 
>
> I suppose that this could be useful. So far I test with block driver 
> traces, i.e. NVMe or SCSI internal traces, just to ensure that we see 
> the requests sent as expected
>

Right.

> This patch adds
>> char 'a' to rwbs field of the trace events if REQ_ATOMIC flag is set in
>> the bio.
>
> All others use uppercase characters, so I suggest that you continue to 
> use that.

It will be good to know on whether only uppercase characters are allowed
or we are good with smallcase characters too? 

> Since 'A' is already used, how about 'U' for untorn? Or 'T' 
> for aTOMic :)
>

If 'a' is not allowed, then we can change it to 'T' maybe.

-ritesh


>> 
>> <W/ REQ_ATOMIC>
>> =================
>> xfs_io-1107    [002] .....   406.206441: block_rq_issue: 8,48 WSa 16384 () 768 + 32 none,0,0 [xfs_io]
>> <idle>-0       [002] ..s1.   406.209918: block_rq_complete: 8,48 WSa () 768 + 32 none,0,0 [0]
>> 
>> <W/O REQ_ATOMIC>
>> ===============
>> xfs_io-1108    [002] .....   411.212317: block_rq_issue: 8,48 WS 16384 () 1024 + 32 none,0,0 [xfs_io]
>> <idle>-0       [002] ..s1.   411.215842: block_rq_complete: 8,48 WS () 1024 + 32 none,0,0 [0]
>> 
>> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>> ---
>>   include/trace/events/block.h | 2 +-
>>   kernel/trace/blktrace.c      | 2 ++
>>   2 files changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/trace/events/block.h b/include/trace/events/block.h
>> index bd0ea07338eb..de538b110ea1 100644
>> --- a/include/trace/events/block.h
>> +++ b/include/trace/events/block.h
>> @@ -11,7 +11,7 @@
>>   #include <linux/tracepoint.h>
>>   #include <uapi/linux/ioprio.h>
>> 
>> -#define RWBS_LEN	8
>> +#define RWBS_LEN	9
>> 
>>   #define IOPRIO_CLASS_STRINGS \
>>   	{ IOPRIO_CLASS_NONE,	"none" }, \
>> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
>> index 3679a6d18934..6badf296ab2b 100644
>> --- a/kernel/trace/blktrace.c
>> +++ b/kernel/trace/blktrace.c
>> @@ -1896,6 +1896,8 @@ void blk_fill_rwbs(char *rwbs, blk_opf_t opf)
>>   		rwbs[i++] = 'S';
>>   	if (opf & REQ_META)
>>   		rwbs[i++] = 'M';
>> +	if (opf & REQ_ATOMIC)
>> +		rwbs[i++] = 'a';
>> 
>>   	rwbs[i] = '\0';
>>   }
>> --
>> 2.48.1
>> 

Re: [RFC] traceevent/block: Add REQ_ATOMIC flag to block trace events

[Ritesh Harjani]

Ritesh Harjani (IBM) <ritesh.list@gmail.com> writes:

> John Garry <john.g.garry@oracle.com> writes:
>
>> On 03/04/2025 06:28, Ritesh Harjani (IBM) wrote:
>>> Filesystems like XFS can implement atomic write I/O using either REQ_ATOMIC
>>> flag set in the bio or via CoW operation. It will be useful if we have a
>>> flag in trace events to distinguish between the two. 
>>
>> I suppose that this could be useful. So far I test with block driver 
>> traces, i.e. NVMe or SCSI internal traces, just to ensure that we see 
>> the requests sent as expected
>>
>
> Right.
>
>> This patch adds
>>> char 'a' to rwbs field of the trace events if REQ_ATOMIC flag is set in
>>> the bio.
>>
>> All others use uppercase characters, so I suggest that you continue to 
>> use that.
>
> It will be good to know on whether only uppercase characters are allowed
> or we are good with smallcase characters too? 
>
>> Since 'A' is already used, how about 'U' for untorn? Or 'T' 
>> for aTOMic :)
>>
>
> If 'a' is not allowed, then we can change it to 'T' maybe.
>

Gentle ping on this.. Any comments/feedback?

It will be good to have these trace events with an identifier to
differentiate between reqs/bios submitted with REQ_ATOMIC flag.

-ritesh

> -ritesh
>
>
>>> 
>>> <W/ REQ_ATOMIC>
>>> =================
>>> xfs_io-1107    [002] .....   406.206441: block_rq_issue: 8,48 WSa 16384 () 768 + 32 none,0,0 [xfs_io]
>>> <idle>-0       [002] ..s1.   406.209918: block_rq_complete: 8,48 WSa () 768 + 32 none,0,0 [0]
>>> 
>>> <W/O REQ_ATOMIC>
>>> ===============
>>> xfs_io-1108    [002] .....   411.212317: block_rq_issue: 8,48 WS 16384 () 1024 + 32 none,0,0 [xfs_io]
>>> <idle>-0       [002] ..s1.   411.215842: block_rq_complete: 8,48 WS () 1024 + 32 none,0,0 [0]
>>> 

Re: [RFC] traceevent/block: Add REQ_ATOMIC flag to block trace events

[Jens Axboe]

On 5/21/25 11:15 PM, Ritesh Harjani (IBM) wrote:
> Ritesh Harjani (IBM) <ritesh.list@gmail.com> writes:
> 
>> John Garry <john.g.garry@oracle.com> writes:
>>
>>> On 03/04/2025 06:28, Ritesh Harjani (IBM) wrote:
>>>> Filesystems like XFS can implement atomic write I/O using either REQ_ATOMIC
>>>> flag set in the bio or via CoW operation. It will be useful if we have a
>>>> flag in trace events to distinguish between the two. 
>>>
>>> I suppose that this could be useful. So far I test with block driver 
>>> traces, i.e. NVMe or SCSI internal traces, just to ensure that we see 
>>> the requests sent as expected
>>>
>>
>> Right.
>>
>>> This patch adds
>>>> char 'a' to rwbs field of the trace events if REQ_ATOMIC flag is set in
>>>> the bio.
>>>
>>> All others use uppercase characters, so I suggest that you continue to 
>>> use that.
>>
>> It will be good to know on whether only uppercase characters are allowed
>> or we are good with smallcase characters too? 
>>
>>> Since 'A' is already used, how about 'U' for untorn? Or 'T' 
>>> for aTOMic :)
>>>
>>
>> If 'a' is not allowed, then we can change it to 'T' maybe.
>>
> 
> Gentle ping on this.. Any comments/feedback?
> 
> It will be good to have these trace events with an identifier to
> differentiate between reqs/bios submitted with REQ_ATOMIC flag.

Just send a v2 with the modified changed. I think 'U' is the most
appropriate one.

-- 
Jens Axboe