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

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

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
>>