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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux