Messages in this thread | | | Date | Fri, 15 Sep 2023 17:13:34 -0400 | From | Steven Rostedt <> | 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@linux-foundation.org> wrote:
> On Fri, 15 Sept 2023 at 13:36, Steven Rostedt <rostedt@goodmis.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
| |