linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] tracing: Remove "__attribute__()" from the type field of event format
Date: Thu, 10 Jul 2025 22:45:20 +0900	[thread overview]
Message-ID: <20250710224520.4eaf514db761ecaaee9bc1e1@kernel.org> (raw)
In-Reply-To: <20250709131107.397a3278@batman.local.home>

On Wed, 9 Jul 2025 13:11:07 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue,  8 Jul 2025 20:54:49 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > With CONFIG_DEBUG_INFO_BTF=y and PAHOLE_HAS_BTF_TAG=y, `__user` is
> > converted to `__attribute__((btf_type_tag("user")))`. In this case,
> > some syscall events have it for __user data, like below;
> > 
> > /sys/kernel/tracing # cat events/syscalls/sys_enter_openat/format
> > name: sys_enter_openat
> > ID: 720
> > format:
> >         field:unsigned short common_type;       offset:0;       size:2; signed:0;
> >         field:unsigned char common_flags;       offset:2;       size:1; signed:0;
> >         field:unsigned char common_preempt_count;       offset:3;       size:1; signed:0;
> >         field:int common_pid;   offset:4;       size:4; signed:1;
> > 
> >         field:int __syscall_nr; offset:8;       size:4; signed:1;
> >         field:int dfd;  offset:16;      size:8; signed:0;
> >         field:const char __attribute__((btf_type_tag("user"))) * filename;      offset:24;      size:8; signed:0;
> >         field:int flags;        offset:32;      size:8; signed:0;
> >         field:umode_t mode;     offset:40;      size:8; signed:0;
> > 
> > .
> > Then the trace event filter fails to set the string acceptable flag
> > (FILTER_PTR_STRING) to the field and rejects setting string filter;
> > 
> >  # echo 'filename.ustring ~ "*ftracetest-dir.wbx24v*"' \
> >     >> events/syscalls/sys_enter_openat/filter  
> >  sh: write error: Invalid argument
> >  # cat error_log
> >  [  723.743637] event filter parse error: error: Expecting numeric field
> >    Command: filename.ustring ~ "*ftracetest-dir.wbx24v*"
> > 
> > Since this __attribute__ makes format parsing complicated and not
> > needed, remove the __attribute__(.*) from the type string.
> 
> Actually, you can do this in update_event_fields() that already does
> this magic for enums as the field length.

Got it. It should be done in the same place.

> 
> And it doesn't free after allocation because it only does the
> allocation for events that will never be freed. For modules, it
> registers the allocated string so it will be freed on unload.

What happen if the tracepoint in module has the __attribute__?
(or enum etc?)


Thanks,
> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

  reply	other threads:[~2025-07-10 13:45 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-08 11:54 [PATCH 0/2] tracing: Fix an event field filter issue Masami Hiramatsu (Google)
2025-07-08 11:54 ` [PATCH 1/2] tracing: Remove "__attribute__()" from the type field of event format Masami Hiramatsu (Google)
2025-07-09  1:06   ` kernel test robot
2025-07-09 16:51     ` Steven Rostedt
2025-07-10 13:47       ` Masami Hiramatsu
2025-07-09 17:11   ` Steven Rostedt
2025-07-10 13:45     ` Masami Hiramatsu [this message]
2025-07-10 13:53       ` Steven Rostedt
2025-07-11  1:44         ` Masami Hiramatsu
2025-07-11  5:37     ` Masami Hiramatsu
2025-07-11 16:03       ` Steven Rostedt
2025-07-12 11:45         ` Masami Hiramatsu
2025-07-12 14:37           ` Steven Rostedt
2025-07-12 15:13             ` Steven Rostedt
2025-07-14 14:14             ` Masami Hiramatsu
2025-07-14 14:32               ` Steven Rostedt
2025-07-08 11:54 ` [PATCH 2/2] tracing: Allocate field->type only if it needs to be sanitized Masami Hiramatsu (Google)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250710224520.4eaf514db761ecaaee9bc1e1@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).