Re: [PATCH 2/2] hugetlbfs: use tracepoints in hugetlbfs functions.

[Hongbo Li <lihongbo22@huawei.com>]

On 2024/7/2 21:30, Mathieu Desnoyers wrote:
> On 2024-07-02 07:55, Hongbo Li wrote:
>>
>>
>> On 2024/7/2 7:49, Steven Rostedt wrote:
>>> On Wed, 12 Jun 2024 09:11:56 +0800
>>> Hongbo Li <lihongbo22@huawei.com> wrote:
>>>
>>>> @@ -934,6 +943,12 @@ static int hugetlbfs_setattr(struct mnt_idmap 
>>>> *idmap,
>>>>       if (error)
>>>>           return error;
>>>> +    trace_hugetlbfs_setattr(inode, dentry->d_name.len, 
>>>> dentry->d_name.name,
>>>> +            attr->ia_valid, attr->ia_mode,
>>>> +            from_kuid(&init_user_ns, attr->ia_uid),
>>>> +            from_kgid(&init_user_ns, attr->ia_gid),
>>>> +            inode->i_size, attr->ia_size);
>>>> +
>>>
>>> That's a lot of parameters to pass to a tracepoint. Why not just pass 
>>> the
>>> dentry and attr and do the above in the TP_fast_assign() logic? That 
>>> would
>>> put less pressure on the icache for the code part.
>>
>> Thanks for reviewing!
>>
>> Some logic such as kuid_t --> uid_t might be reasonable obtained in 
>> filesystem layer. Passing the dentry and attr will let trace know the 
>> meaning of structure, perhaps tracepoint should not be aware of the
>> members of these structures as much as possible.
> 
> As maintainer of the LTTng out-of-tree kernel tracer, I appreciate the
> effort to decouple instrumentation from the subsystem instrumentation,
> but as long as the structure sits in public headers and the global
> variables used within the TP_fast_assign() logic (e.g. init_user_ns)
> are export-gpl, this is enough to make it easy for tracer integration
Thank you for your friendly elaboration and suggestion!
I will update this part based on your suggestion in next version.

Thanks,
Hongbo
> and it keeps the tracepoint caller code footprint to a minimum.
> 
> The TRACE_EVENT definitions are specific to the subsystem anyway,
> so I don't think it matters that the TRACE_EVENT() need to access
> the dentry and attr structures.
> 
> So I agree with Steven's suggestion. However, just as a precision,
> I suspect it will have mainly an impact on code size, but not
> necessarily on icache footprint, because it will shrink the code
> size within the tracepoint unlikely branch (cold instructions).
> 
> Thanks,
> 
> Mathieu
> 
>>
>> Thanks,
>> Hongbo
>>
>>>
>>> -- Steve
>>>
>