Re: [PATCH][RESEND] fuse: add simple request tracepoints

[Bernd Schubert <bernd.schubert@fastmail.fm>]

On 7/5/24 16:52, Josef Bacik wrote:
> On Thu, Jul 04, 2024 at 07:20:16PM +0200, Bernd Schubert wrote:
>>
>>
>> On 7/3/24 16:38, Josef Bacik wrote:
>>> I've been timing various fuse operations and it's quite annoying to do
>>> with kprobes.  Add two tracepoints for sending and ending fuse requests
>>> to make it easier to debug and time various operations.
>>
>> Thanks, this is super helpful.
>>
>> [...]
>>>
>>> +	EM( FUSE_STATX,			"FUSE_STATX")		\
>>> +	EMe(CUSE_INIT,			"CUSE_INIT")
>>> +
>>> +/*
>>> + * This will turn the above table into TRACE_DEFINE_ENUM() for each of the
>>> + * entries.
>>> + */
>>> +#undef EM
>>> +#undef EMe
>>> +#define EM(a, b)	TRACE_DEFINE_ENUM(a);
>>> +#define EMe(a, b)	TRACE_DEFINE_ENUM(a);
>>
>>
>> I'm not super familiar with tracepoints and I'm a bit list why "EMe" is
>> needed
>> in addition to EM? CUSE_INIT is just another number?
> 
> This is just obnoxious preprocessor abuse, so you're right this first iteration
> of EMe() is the same as EM(), but if you look right below that you have
> 
> /* Now we redfine it with the table that __print_symbolic needs. */
> #undef EM
> #undef EMe
> #define EM(a, b)        {a, b},
> #define EMe(a, b)       {a, b}
> 
> so later when we do
> 
> __print_symbolic(__entry->opcode, OPCODES)
> 
> OPCODES gets turned intoo
> 
> __print_symbolic(__entry->opcode,
> 		{FUSE_LOOKUP, "FUSE_LOOKUP"},{...},{CUSE_INIT, "CUSE_INIT"})
> 
> it's subtle and annoying, but the cleanest way to have these big opcode tables
> that are easy to add/remove stuff from for clean output.  Thanks,

Ah, I had missed the comma difference, sorry for the noise!
Looks good and very useful to me (I'm also using it now).


Reviewed-by: Bernd Schubert <bschubert@ddn.com>