linux-kernel - Re: [GIT PULL] tracing: Add eventfs file to help with debugging any more issues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230915171334.5c231ca7@gandalf.local.home>
Date:   Fri, 15 Sep 2023 17:13:34 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ajay Kaher <akaher@...are.com>
Subject: Re: [GIT PULL] tracing: Add eventfs file to help with debugging any
 more issues

On Fri, 15 Sep 2023 13:50:17 -0700
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Fri, 15 Sept 2023 at 13:36, Steven Rostedt <rostedt@...dmis.org> wrote:
> >
> > I'm OK with it not going in now, but instead I'll wrap an ifdef around it
> > and move it to my queue for the next merge window. I still would like to
> > keep these "what's going on internally" available, as I'll ask people to
> > enable them when they report an issue.  
> 
> Honestly, you copied the pattern from the /proc filesystem.

I didn't actually copy it, even though the /proc filesystem does something
similar (I didn't even know that it did until I presented this idea for
eventfs in LSFMM, and someone told me that /proc did so too).

I tried to look at how /proc does things and I couldn't really use it as
easily, because proc uses its own set of "proc_ops", and I had some
different requirements.

> 
> The /proc filesyustem is widely used and has never had this kind of
> random debugging code in mainline.

Again, it is implemented differently.

> 
> Seriously, that eventfs_file thing is not worthy of this kind of
> special debug code.
> 
> That debug code seems to be approaching the same order of size as all
> the code evetfs_file code itself is.

You mean the event_show.c code?

> 
> There's a point where this kind of stuff just becomes ridiculous. At
> least wait until there's a *reason* to debug a simple linked list of
> objects.
> 
> If you have a hard time figuring out what the eventfs entries are,
> maybe you should just have made "iterate_shared" show them, and then
> you could use fancy tools like "ls" to see what the heck is up in that
> directory?
> 

I was more interested in what did not exist than what existed. I wanted to
make sure that things were cleaned up properly. One of my tests that I used
was to do a: find /sys/kernel/tracing/events, and then run my ring_buffer
memory size stress test (that keeps increasing the size of the ring buffer
to make sure it fails safely when it runs out of memory). Then I check to make
sure all the unused dentries and inodes were reclaimed nicely, as they hang
around until a reclaim is made.

It did prove useful for the initial debugging, but it also helped a lot for
the new code I have saved for the next merge window. That code changes the
internal interface quite drastically.

The current code has meta data for every file in the eventfs (defined by the
eventfs_file structure). The new code has meta data only for the events
themselves (which map to the directories) and I remove the eventfs_file
entirely. It uses a callback from the eventfs code to create the dentries
and inodes of the files on the fly (there's no meta data representing the
individual files).

  https://lore.kernel.org/linux-trace-kernel/20230914163535.269645249@goodmis.org/

Now I use this debug file to know what files are added, and more
importantly, not added.

Are you entirely against this file, or is it fine if it's just wrapped
around an CONFIG_EVENTFS_DEBUG?

-- Steve