mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Beau Belgrave <beaub@linux.microsoft.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	linux-trace-devel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] user_events: Enable user processes to create and write to trace events
Date: Tue, 12 Oct 2021 21:18:52 -0400	[thread overview]
Message-ID: <20211012211852.2bbf921b@oasis.local.home> (raw)
In-Reply-To: <20211011162523.GA1542@kbox>

On Mon, 11 Oct 2021 09:25:23 -0700
Beau Belgrave <beaub@linux.microsoft.com> wrote:
> 
> Yes, in my mind there are two options to avoid kernel memory usage
> per-event.
> 
> 1.
> We have a an array per file struct that is independently ref-counted.
> This is required to ensure lifetime requirements and to ensure user code
> cannot access other user events that might have been free'd outside of
> the lifetime and cause a kernel crash.
> 
> This approach also requires 2 int's to be returned, 1 for the status
> page the other a local index for the write into the above array per-file
> struct.
> 
> This is likely the most complex method due to it's lifetime and RCU
> synchronization requirements. However, it represents the least memory to
> both kernel and user space.

Does it require RCU synchronization as the updates only happen from
user space. But is this for the writing of the event? You want a
separate fd for each event to write to, instead of saying you have
another interface to write and just pass the given id?

> 
> 2.
> We have a anon_inode FD that gets installed into the user process and
> returned via the ioctl from user_events tracefs file. The file struct
> backing the FD is shared by all user mode processes for that event. Like
> having an inject/marker file per-event in the user_events subsystem.
> 
> This approach requires an FD returned and either an int for the status
> page or the returend FD could expose the ID via another IOCTL being
> issued.
> 
> This is the simplest method since the FD manages the lifetime, when FD
> is released so is the shared file struct. Kernel side memory is reduced
> to only unique events that are actively being used. There is no RCU or
> synchronization beyond the FD lifetime. The user mode processes does
> incur an FD per-event within their file description table. So they
> events charge against their FD per-process limit (not necessarily a bad
> thing).
> 
> This also seems to follow the pre-existing patterns of tracefs
> (trace_marker, inject, format, etc all have a shared file available to
> user-processes that have been granted access). For our case, we want
> that, but we want it on a access boundary to who all have access to the
> user_events_* tracefs files. We don't want to open up all of tracefs
> widely.
> 
> > > I want to make
> > > sure the complexity is worth it. Is the overhead of an FD per event in
> > > user space too much?  
> > 
> > It depends on the use case, how much events you wants to use with
> > the user-events. If there are hundreds of the evets, that will consume
> > kernel resources and /proc/*/fd/ will be filled with the event's fds.
> > But if there is a few events, I think no problem.
> >   
> In our own use case this will be low due to the way we plan to use the
> events. However, I am not sure others will follow that :)

I will say, whenever we say this will only have a "few", if it becomes
useful, it will end up having many.

-- Steve

  reply	other threads:[~2021-10-13  1:18 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-05 22:44 Beau Belgrave
2021-10-06 16:28 ` Masami Hiramatsu
2021-10-06 17:56   ` Beau Belgrave
2021-10-07 14:17     ` Masami Hiramatsu
2021-10-07 16:22       ` Beau Belgrave
2021-10-07 23:12         ` Masami Hiramatsu
2021-10-08  0:05           ` Beau Belgrave
2021-10-08  9:22             ` Masami Hiramatsu
2021-10-11 16:25               ` Beau Belgrave
2021-10-13  1:18                 ` Steven Rostedt [this message]
2021-10-13 16:50                   ` Beau Belgrave
2021-10-13 17:11                     ` Steven Rostedt
2021-10-13 17:17                       ` Beau Belgrave
2021-10-13 17:27                         ` Steven Rostedt
2021-10-13 15:21                 ` Masami Hiramatsu
2021-10-13 15:40                   ` Steven Rostedt
2021-10-14 12:21                     ` Masami Hiramatsu
2021-10-13 16:56                   ` Beau Belgrave
2021-10-06 16:54 ` Steven Rostedt
2021-10-06 17:27   ` Beau Belgrave
2021-10-06 17:44     ` Steven Rostedt
2021-10-08 13:11 ` Peter.Enderborg
2021-10-08 16:09   ` Beau Belgrave

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=20211012211852.2bbf921b@oasis.local.home \
    --to=rostedt@goodmis.org \
    --cc=beaub@linux.microsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=mhiramat@kernel.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

all inboxes | Powered by JetHome®