Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-31 Thread Steven Rostedt
On Wed, 31 Jan 2024 10:58:47 -0500 Steven Rostedt wrote: > BTW, I ran my full test suite on your patches with the below updates and it > all passed. Note, I did not run the "bisectable" portion of my test. That > is, the part that runs tests on each patch in the ser

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-31 Thread Steven Rostedt
On Wed, 31 Jan 2024 10:58:47 -0500 Steven Rostedt wrote: > @@ -788,6 +717,7 @@ static void init_once(void *foo) > { > struct tracefs_inode *ti = (struct tracefs_inode *) foo; > > + memset(ti, 0, sizeof(*ti)); > inode_init_once(>vfs_inode); > } >

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-31 Thread Steven Rostedt
On Tue, 30 Jan 2024 11:54:56 -0800 Linus Torvalds wrote: > If you do run the full tracefs tests on the whole series, and there > are no other major problems, I'll happily take it all for 6.8. And > yes, even mark it for stable. I think the other bugs are much harder > to hit, but I do think they

Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

2024-01-31 Thread Steven Rostedt
On Wed, 31 Jan 2024 07:57:40 -0500 Steven Rostedt wrote: > static int instance_rmdir(const char *name) > { > struct trace_array *tr; > int ret; > > mutex_lock(_mutex); Note, event_mutex prevents dynamic events from being created. No kprobe

Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

2024-01-31 Thread Steven Rostedt
On Tue, 30 Jan 2024 22:20:24 -0800 Linus Torvalds wrote: > On Tue, 30 Jan 2024 at 21:57, Linus Torvalds > wrote: > > > > Ugh. > > Oh, and double-ugh on that tracefs_syscall_mkdir/rmdir(). I hate how > it does that "unlock and re-lock inode" thing. I'd figured you'd like that one. > > It's

Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 21:25:30 -0800 Linus Torvalds wrote: > > Does this work: > > > > d_invalidate(dentry); > > It does, but it's basically irrelevant with the d_revalidate approach. > > Basically, once you have d_revalidate(), the unhashing happens there, > and it's just extra work

Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 11:03:54 -0800 Linus Torvalds wrote: > +/* > + * eventfs_inode reference count management. > + * > + * NOTE! We count only references from dentries, in the > + * form 'dentry->d_fsdata'. There are also references from > + * directory inodes ('ti->private'), but the dentry

[PATCH v2] tracefs: Zero out the tracefs_inode when allocating it

2024-01-30 Thread Steven Rostedt
From: "Steven Rostedt (Google)" eventfs uses the tracefs_inode and assumes that it's already initialized to zero. That is, it doesn't set fields to zero (like ti->private) after getting its tracefs_inode. This causes bugs due to stale values. Just initialize the entire stru

[PATCH v2 (3/6)] tracefs: dentry lookup crapectomy

2024-01-30 Thread Steven Rostedt
es becoming negative dentries. Which meant that any other lookup would possibly return ENOENT if it saw that negative dentry before the data rwas then later filled in. You could see it in the immesnse amount of nonsensical code that didn't actually just do lookups. Signed-off-by: Linus Torvalds S

Re: [PATCH 3/6] tracefs: dentry lookup crapectomy

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 23:26:21 + Al Viro wrote: > On Tue, Jan 30, 2024 at 11:03:52AM -0800, Linus Torvalds wrote: > > The dentry lookup for eventfs files was very broken, and had lots of > > signs of the old situation where the filesystem names were all created > > statically in the dentry

Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 15:06:13 -0800 Linus Torvalds wrote: > On Tue, 30 Jan 2024 at 14:56, Linus Torvalds > wrote: > > > > With that, the base size of 'struct eventfs_inode' actually becomes 96 > > bytes for me. > > It can be shrunk some more. > > The field ordering is suboptimal. Pointers

Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 14:58:26 -0800 Linus Torvalds wrote: > On Tue, 30 Jan 2024 at 14:55, Steven Rostedt wrote: > > > > Remember, the files don't have an ei allocated. Only directories. > > Crossed emails. > > > I then counted the length of the strin

Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 13:52:05 -0800 Linus Torvalds wrote: > On Tue, 30 Jan 2024 at 12:55, Steven Rostedt wrote: > > > > I'm going to be putting back the ei->name pointer as the above actually > > adds more memory usage. > > I did it mainly because I hate having

Re: [PATCH 6/6] eventfs: clean up dentry ops and add revalidate function

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 11:03:55 -0800 Linus Torvalds wrote: > Another thing that might be worth doing is to make all eventfs lookups > mark their dentries as not worth caching. We could do that with > d_delete(), but the DCACHE_DONTCACHE flag would likely be even better. > > As it is, the

Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 11:03:54 -0800 Linus Torvalds wrote: > -static void free_ei(struct eventfs_inode *ei) > +static inline struct eventfs_inode *alloc_ei(const char *name) > { > - kfree_const(ei->name); > - kfree(ei->d_children); > - kfree(ei->entry_attrs); > - kfree(ei); > +

[PATCH v2] eventfs: Initialize the tracefs inode properly

2024-01-30 Thread Steven Rostedt
emove eventfs_file and just use eventfs_inode") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-lkp/202401291043.e62e89dc-oliver.s...@intel.com Signed-off-by: Linus Torvalds Signed-off-by: Steven Rostedt (Google) --- Changes since v1: https://lore.kernel.org/linux-tr

[PATCH] tracefs: Zero out the tracefs_inode when allocating it

2024-01-30 Thread Steven Rostedt
From: "Steven Rostedt (Google)" eventfs uses the tracefs_inode and assumes that it's already initialized to zero. That is, it doesn't set fields to zero (like ti->private) after getting its tracefs_inode. This causes bugs due to stale values. Just initialize the entire stru

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 11:54:56 -0800 Linus Torvalds wrote: > On Tue, 30 Jan 2024 at 11:37, Steven Rostedt wrote: > > > > Do you want me to put this in my urgent branch and mark them for stable, > > and then send them for 6.8? > > Hmm. I think the only one

Re: [PATCH 2/6] eventfsfs: initialize the tracefs inode properly

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 14:48:02 -0500 Steven Rostedt wrote: > The ti is allocated from fs/tracefs/inode.c that has: > > static struct inode *tracefs_alloc_inode(struct super_block *sb) > { > struct tracefs_inode *ti; > > ti = kmem_cache_alloc(tracefs_inode_ca

Re: [PATCH 2/6] eventfsfs: initialize the tracefs inode properly

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 11:03:51 -0800 Linus Torvalds wrote: > @@ -328,7 +328,9 @@ static struct dentry *create_file(const char *name, > umode_t mode, > inode->i_ino = EVENTFS_FILE_INODE_INO; > > ti = get_tracefs(inode); > - ti->flags |= TRACEFS_EVENT_INODE; > + ti->flags =

Re: [PATCH 2/6] eventfsfs: initialize the tracefs inode properly

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 11:03:51 -0800 Linus Torvalds wrote: > The tracefs-specific fields in the inode were not initialized before the > inode was exposed to others through the dentry with 'd_instantiate()'. > > And the ->flags file was initialized incorrectly with a '|=', when the > old value was

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 11:19:01 -0800 Linus Torvalds wrote: > On Tue, 30 Jan 2024 at 10:23, Steven Rostedt wrote: > > > > I know you don't send patches inlined anymore, which is a shame, because > > patchwork takes care of all the administering when patches are inlined,

Re: [PATCH 2/4] tracing/user_events: Introduce multi-format events

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 10:05:15 -0800 Beau Belgrave wrote: > On Mon, Jan 29, 2024 at 09:24:07PM -0500, Steven Rostedt wrote: > > On Mon, 29 Jan 2024 09:29:07 -0800 > > Beau Belgrave wrote: > > > > > Thanks, yeah ideally we wouldn't use special characters. &g

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-30 Thread Steven Rostedt
On Mon, 29 Jan 2024 19:56:52 -0800 Linus Torvalds wrote: > [0001-tracefs-avoid-using-the-ei-dentry-pointer-unnecessar.patch > text/x-patch (3561 bytes)] > > [0002-eventfsfs-initialize-the-tracefs-inode-properly.patch text/x-patch > (2548 bytes)] Ah these two are new. -- Steve

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-30 Thread Steven Rostedt
On Mon, 29 Jan 2024 19:56:52 -0800 Linus Torvalds wrote: > [0005-eventfs-get-rid-of-dentry-pointers-without-refcounts.patch > text/x-patch (15652 bytes)] I missed this email when I wrote basically the same thing :-p It was sent just as I went to bed and this morning I mistaken it as an

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 08:55:51 -0800 Linus Torvalds wrote: > On Tue, 30 Jan 2024 at 08:49, Steven Rostedt wrote: > > > > - On removal, I got rid of the SRCU callback and the work queue. > > Instead, I find the dentry of the current eventfs_inode that is being > >

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 09:39:42 -0500 Steven Rostedt wrote: > I may try something that will still let me get rid of the ei->dentry. This appears to work, but like always, I may have missed something. I need to add debugging (printks) to make sure the that I don't leave any dangling dentries

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 01:12:05 -0800 Linus Torvalds wrote: > On Tue, 30 Jan 2024 at 00:43, Linus Torvalds > wrote: > > > > I'll go back to bed, but I think the fix is something trivial like this: > > Almost. > > > + result = ERR_PTR(ENOENT); > > That needs a '-' in front of the

Re: [PATCH v9] bus: mhi: host: Add tracing support

2024-01-30 Thread Steven Rostedt
On Tue, 30 Jan 2024 13:41:52 +0530 Manivannan Sadhasivam wrote: > So same trace will get printed for both mhi_channel_command_start() and > mhi_channel_command_end()? The trace output will also include the tracepoint name. That is, it will have the same content but will be preceded with:

Re: [PATCH 2/4] tracing/user_events: Introduce multi-format events

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 09:29:07 -0800 Beau Belgrave wrote: > Thanks, yeah ideally we wouldn't use special characters. > > I'm not picky about this. However, I did want something that clearly > allowed a glob pattern to find all versions of a given register name of > user_events by user programs

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 16:01:25 -0800 Linus Torvalds wrote: > I'll go see what's up with the "create it again" case - I don't > immediately see what's wrong. Interesting. I added a printk in the lookup, and just did this: # cd /sys/kernel/tracing # ls events/kprobes And it showed that it tried

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 17:47:32 -0500 Steven Rostedt wrote: > > And I hope there aren't any other stupid things I missed like that. > > Well the preliminary tests pass with this added to your patch: Spoke too soon. The later tests started failing. It fails on creating a kpro

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 14:42:47 -0800 Linus Torvalds wrote: > @@ -324,7 +322,7 @@ static struct dentry *lookup_file(struct dentry *dentry, > ti->flags = TRACEFS_EVENT_INODE; > ti->private = NULL; // Directories have 'ei', files > not > > -

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 14:35:37 -0800 Linus Torvalds wrote: > And I hope there aren't any other stupid things I missed like that. Well the preliminary tests pass with this added to your patch: diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index cd6de322..ad11063bdd53 100644

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 12:51:59 -0800 Linus Torvalds wrote: > [0004-tracefs-dentry-lookup-crapectomy.patch text/x-patch (11761 bytes)] I had to add: diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index cd6de322..89897d934302 100644 --- a/fs/tracefs/event_inode.c +++

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 12:51:59 -0800 Linus Torvalds wrote: > [0001-tracefs-remove-stale-update_gid-code.patch text/x-patch (2612 bytes)] Oh, I already applied this and even sent you a pull request with it. https://lore.kernel.org/all/20240128223151.2dad6...@rorschach.local.home/ -- Steve

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 12:51:59 -0800 Linus Torvalds wrote: > End result: what simple_lookup() does is say "oh, you didn't have the > file, so it's by definition a negative dentry", and thus all it does > is to do "d_add(dentry, NULL)". > > Anyway, removing this was painful. I initially thought

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 11:51:52 -0800 Linus Torvalds wrote: > On Mon, 29 Jan 2024 at 11:24, Linus Torvalds > wrote: > > > > So the patch was completely broken. Here's the one that should > > actually compile (although still not actually *tested*). > > Note that this fixes the d_instantiate()

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 12:44:36 -0500 Steven Rostedt wrote: > On Mon, 29 Jan 2024 09:40:06 -0800 > Linus Torvalds wrote: > > > Now, I do agree that your locking is strange, and that should be fixed > > *too*, but I think the above is the "right" fix for this particu

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Mon, 29 Jan 2024 09:40:06 -0800 Linus Torvalds wrote: > Now, I do agree that your locking is strange, and that should be fixed > *too*, but I think the above is the "right" fix for this particular > issue. Would you be OK if I did both as a "fix"? -- Steve

Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address

2024-01-29 Thread Steven Rostedt
On Sun, 28 Jan 2024 20:36:12 -0800 Linus Torvalds wrote: > End result: the d_instantiate() needs to be done *after* the inode has > been fully filled in. > > Alternatively, you could > > (a) not drop the eventfs_mutex around the create_dir() call > > (b) take the eventfs_mutex around all

Re: [RESEND] [PATCH] eventfs: Have inodes have unique inode numbers

2024-01-26 Thread Steven Rostedt
On Fri, 26 Jan 2024 15:24:17 -0500 Mathieu Desnoyers wrote: > On 2024-01-26 15:12, Steven Rostedt wrote: > [...] > > diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c > > index e1b172c0e091..2187be6d7b23 100644 > > --- a/fs/tracefs/inode.c > > +++ b/fs/tracefs/

[RESEND] [PATCH] eventfs: Have inodes have unique inode numbers

2024-01-26 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Linus suggested to use the same inode numbers to make it easier to implement getdents(), as it was creating inodes just for generating a unique and consistent inode number. Linus suggested to just use the same inode for all files and directori

Re: [PATCH 2/4] tracing/user_events: Introduce multi-format events

2024-01-26 Thread Steven Rostedt
On Fri, 26 Jan 2024 11:10:07 -0800 Beau Belgrave wrote: > > OK, so the each different event has suffixed name. But this will > > introduce non C-variable name. > > > > Steve, do you think your library can handle these symbols? It will > > be something like "event:[1]" as the event name. > >

Re: [PATCH] eventfs: Give files a default of PAGE_SIZE size

2024-01-26 Thread Steven Rostedt
On Fri, 26 Jan 2024 11:06:33 -0800 Linus Torvalds wrote: > On Fri, 26 Jan 2024 at 10:41, Steven Rostedt wrote: > > > > Fine, but I still plan on sending you the update to give all files unique > > inode numbers. If it screws up tar, it could possibly screw up something

Re: [PATCH] eventfs: Give files a default of PAGE_SIZE size

2024-01-26 Thread Steven Rostedt
On Fri, 26 Jan 2024 10:31:07 -0800 Linus Torvalds wrote: > On Fri, 26 Jan 2024 at 10:18, Steven Rostedt wrote: > > > > By following what sysfs does, and give files a default size of PAGE_SIZE, > > it allows the tar to work. No event file is greater than PAGE_SIZE. >

[PATCH] eventfs: Give files a default of PAGE_SIZE size

2024-01-26 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The sqlhist utility[1] (or the new trace-cmd sqlhist command[2]) creates the commands of a synthetic event based on the files in the tracefs/events directory. When creating these commands for an embedded system, it is asked to copy the files to the temp

Re: [PATCH] tracing/trigger: Fix to return error if failed to alloc snapshot

2024-01-26 Thread Steven Rostedt
On Fri, 26 Jan 2024 09:42:58 +0900 "Masami Hiramatsu (Google)" wrote: > From: Masami Hiramatsu (Google) > > Fix register_snapshot_trigger() to return error code if it failed to > allocate a snapshot instead of 0 (success). Unless that, it will register > snapshot trigger without an error. > >

Re: [PATCH] ring-buffer: Simplify reservation with try_cmpxchg() loop

2024-01-25 Thread Steven Rostedt
On Thu, 25 Jan 2024 16:18:37 -0500 Mathieu Desnoyers wrote: > > > > This is how you are able to avoid the "before/after" logic I have, as > > the race is automatically detected. The least significant bits of the > > timestamp is ignored for the event delta calculation. > > Not quite, as I

Re: [PATCH v2 2/3] tracing: initialize trace_seq buffers

2024-01-25 Thread Steven Rostedt
, the change log should be: In order to extend trace_seq into being dynamic, the struct trace_seq will no longer be valid if simply set to zero. Call trace_seq_init() for all trace_seq when they are first created. > > Suggested-by: Steven Rostedt > Signed-off-by: Ricardo B. Marlie

Re: [PATCH RFC v3 00/35] Add support for arm64 MTE dynamic tag storage reuse

2024-01-25 Thread Steven Rostedt
On Thu, 25 Jan 2024 16:42:21 + Alexandru Elisei wrote: > include/trace/events/cma.h| 59 ++ > include/trace/events/mmflags.h| 5 +- I know others like being Cc'd on every patch in a series, but I'm not about to trudge through 35 patches to review trace

Re: [PATCH] tracing: Include PPIN in mce_record tracepoint

2024-01-24 Thread Steven Rostedt
On Wed, 24 Jan 2024 10:57:08 +0100 Borislav Petkov wrote: > On Tue, Jan 23, 2024 at 08:38:53PM -0500, Steven Rostedt wrote: > > Yes, rasdaemon uses libtraceevent (or a copy of it internally) that > > reads the format file to find fields. You can safely add fields to the > >

Re: [PATCH] tracing: Include PPIN in mce_record tracepoint

2024-01-23 Thread Steven Rostedt
On Tue, 23 Jan 2024 19:29:52 -0600 "Naik, Avadhut" wrote: > > But some questions: > > > > 1) Are tracepoints a user visible ABI? Adding a new field in the middle > > feels like it might be problematic. I asked this question many years > > ago and St

Re: [PATCH v12 2/6] ring-buffer: Introducing ring-buffer mapping functions

2024-01-23 Thread Steven Rostedt
On Tue, 23 Jan 2024 17:48:17 + Vincent Donnefort wrote: > > > + * @subbufs_touched: Number of subbufs that have been filled. > > > + * @subbufs_lost:Number of subbufs lost to overrun. > > > + * @subbufs_read:Number of subbufs that have been read. > > > > Do we actually

Re: [PATCH v12 2/6] ring-buffer: Introducing ring-buffer mapping functions

2024-01-23 Thread Steven Rostedt
On Tue, 23 Jan 2024 11:07:53 + Vincent Donnefort wrote: > index ..5468afc94be7 > --- /dev/null > +++ b/include/uapi/linux/trace_mmap.h > @@ -0,0 +1,44 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +#ifndef _TRACE_MMAP_H_ > +#define _TRACE_MMAP_H_ > + >

Re: [PATCH v3 4/7] tracing/probes: support '%pD' type for print struct file's name

2024-01-22 Thread Steven Rostedt
On Tue, 23 Jan 2024 10:56:05 +0800 Ye Bin wrote: > Similar to '%pD' for printk, use '%pD' for print struct file's name. > > Signed-off-by: Ye Bin > --- > kernel/trace/trace_probe.c | 41 -- > 1 file changed, 26 insertions(+), 15 deletions(-) > > diff --git

Re: [PATCH] percpu: improve percpu_alloc_percpu_fail event trace

2024-01-22 Thread Steven Rostedt
On Tue, 23 Jan 2024 09:44:43 +0800 George Guo wrote: > There are two reasons of percpu_alloc failed without warnings: > > 1. do_warn is false > 2. do_warn is true and warn_limit is reached the limit. Yes I know the reasons. > > Showing do_warn and warn_limit makes things simple, maybe dont

Re: [PATCH] tracing: add trace_seq_reset function

2024-01-22 Thread Steven Rostedt
On Mon, 22 Jan 2024 19:45:41 -0300 "Ricardo B. Marliere" wrote: > > Perhaps we need a: > > > > if (WARN_ON_ONCE(!s->seq.size)) > > seq_buf_init(>seq, s->buffer, TRACE_SEQ_BUFFER_SIZE); > > else > > seq_buf_clear(>seq); > > > > > > in the trace_seq_reset() to

Re: [PATCH] tracing: add trace_seq_reset function

2024-01-22 Thread Steven Rostedt
On Mon, 22 Jan 2024 15:22:25 -0300 "Ricardo B. Marliere" wrote: > Currently, trace_seq_init may be called many times with the intent of > resetting the buffer. Add a function trace_seq_reset that does that and > replace the relevant occurrences to use it instead. > Hi Ricardo! It's also OK to

[PATCH] eventfs: Save directory inodes in the eventfs_inode structure

2024-01-22 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The eventfs inodes and directories are allocated when referenced. But this leaves the issue of keeping consistent inode numbers and the number is only saved in the inode structure itself. When the inode is no longer referenced, it can be freed. Whe

Re: [PATCH] percpu: improve percpu_alloc_percpu_fail event trace

2024-01-22 Thread Steven Rostedt
On Mon, 22 Jan 2024 15:36:29 +0800 George Guo wrote: > From: George Guo > > Add do_warn, warn_limit fields to the output of the > percpu_alloc_percpu_fail ftrace event. > > This is required to percpu_alloc failed with no warning showing. You mean to state; In order to know why

Re: [PATCH v2 4/7] tracing/probes: support '%pD' type for print struct file's name

2024-01-22 Thread Steven Rostedt
On Mon, 22 Jan 2024 15:40:12 +0800 Ye Bin wrote: > Similar to '%pD' for printk, use '%pD' for print struct file's name. > > Signed-off-by: Ye Bin > --- > kernel/trace/trace_probe.c | 41 -- > 1 file changed, 26 insertions(+), 15 deletions(-) > > diff --git

Re: [PATCH v2 2/7] tracing/probes: add traceprobe_expand_dentry_args() helper

2024-01-22 Thread Steven Rostedt
On Mon, 22 Jan 2024 15:40:10 +0800 Ye Bin wrote: > Add traceprobe_expand_dentry_args() to expand dentry args. this API is > prepare to support "%pd" print format for kprobe. > > Signed-off-by: Ye Bin > --- > kernel/trace/trace_probe.c | 34 ++ >

Re: [PATCH v2 1/7] string.h: add str_has_suffix() helper for test string ends with specify string

2024-01-22 Thread Steven Rostedt
On Mon, 22 Jan 2024 15:40:09 +0800 Ye Bin wrote: > str_has_suffix() is test string if ends with specify string. > > Signed-off-by: Ye Bin > --- > include/linux/string.h | 20 > 1 file changed, 20 insertions(+) > > diff --git a/include/linux/string.h

Re: [PATCH] ring-buffer: Simplify reservation with try_cmpxchg() loop

2024-01-20 Thread Steven Rostedt
On Sat, 20 Jan 2024 08:47:13 -0500 Steven Rostedt wrote: > > I would also consider reducing code complexity as a worthwhile metric > > in addition to speed. It makes it easier to extend in the future, > > easier to understand for reviewers, and subtle bugs are less l

Re: [PATCH] ring-buffer: Simplify reservation with try_cmpxchg() loop

2024-01-20 Thread Steven Rostedt
On Fri, 19 Jan 2024 20:49:36 -0500 Mathieu Desnoyers wrote: > >> Let's say we have the following ktime_get() values (monotonic timestamp > >> value) for > >> a sequence of events: > >> > >> Timestamp (Hex)Encoding in the > >> trace > >> > >> Packet

Re: [PATCH] ring-buffer: Simplify reservation with try_cmpxchg() loop

2024-01-19 Thread Steven Rostedt
On Fri, 19 Jan 2024 15:56:21 -0500 Mathieu Desnoyers wrote: > > So when an overflow happens, you just insert a timestamp, and then events > > after that is based on that? > > No. Let's use an example to show how it works. > > For reference, LTTng uses 5-bit for event ID and 27-bit for

[PATCH] ring-buffer: Add writer_loops to stat file

2024-01-19 Thread Steven Rostedt
From: "Steven Rostedt (Google)" As the ring buffer never had a cmpxchg loop until the reserving logic added one, add a statistic on how many times it happens. This can be used to make sure there's not any issues because the writer is retrying. ~# cat /sys/kernel/tracing/per_cpu/

Re: [PATCH 0/3] support '%pd' and '%pD' for print file name

2024-01-19 Thread Steven Rostedt
On Fri, 19 Jan 2024 23:43:56 +0900 Masami Hiramatsu (Google) wrote: > Thanks for your proposal! > > Generically, I think this type of hack is not good for the tracing > because there are already some ways to do that. e.g. > - Use perf probe to specify dentry->name:string or file->name:string

Re: [PATCH] ring-buffer: Simplify reservation with try_cmpxchg() loop

2024-01-19 Thread Steven Rostedt
On Fri, 19 Jan 2024 09:40:27 -0500 Mathieu Desnoyers wrote: > On 2024-01-18 18:12, Steven Rostedt wrote: > > From: "Steven Rostedt (Google)" > > > > Instead of using local_add_return() to reserve the ring buffer data, > > Mathieu Desnoyers suggested using

[PATCH] ring-buffer: Simplify reservation with try_cmpxchg() loop

2024-01-18 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Instead of using local_add_return() to reserve the ring buffer data, Mathieu Desnoyers suggested using local_cmpxchg(). This would simplify the reservation with the time keeping code. Although, it does not get rid of the double time stamps (be

Re: [PATCH v2] kprobes: Use synchronize_rcu_tasks_rude in kprobe_optimizer

2024-01-17 Thread Steven Rostedt
On Thu, 18 Jan 2024 02:18:42 + Chen Zhongjin wrote: > There is a deadlock scenario in kprobe_optimizer(): > > pid A pid B pid C > kprobe_optimizer()do_exit() perf_kprobe_init() > mutex_lock(_mutex)

[PATCH v3 2/2] eventfs: Do not create dentries nor inodes in iterate_shared

2024-01-16 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The original eventfs code added a wrapper around the dcache_readdir open callback and created all the dentries and inodes at open, and increment their ref count. A wrapper was added around the dcache_readdir release function to decrement all the

[PATCH v3 0/2] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt
[ subject is still wrong, but is to match v2, see patch 2 for correct subject ] Changes since v2: https://lore.kernel.org/all/20240116211217.968123...@goodmis.org/ Implemented Linus's suggestion to just change the iterate_shared to use the hard coded inodes. Steven Rostedt (Google) (2

[PATCH v3 1/2] eventfs: Have the inodes all for files and directories all be the same

2024-01-16 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The dentries and inodes are created in the readdir for the sole purpose of getting a consistent inode number. Linus stated that is unnecessary, and that all inodes can have the same inode number. For a virtual file system they are pretty meaningless. I

Re: [PATCH v2 2/2] eventfs: Create list of files and directories at dir open

2024-01-16 Thread Steven Rostedt
On Tue, 16 Jan 2024 13:39:38 -0800 Linus Torvalds wrote: > I don't understand why your still insist on this pointless open wrapper. I just liked the consistency of it. > > Just do this all at iterate time. No open wrappers. No nothing. Just > iterate over the days structures your have. > >

[PATCH v2 2/2] eventfs: Create list of files and directories at dir open

2024-01-16 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The original eventfs code added a wrapper around the dcache_readdir open callback and created all the dentries and inodes at open, and increment their ref count. A wrapper was added around the dcache_readdir release function to decrement all the

[PATCH v2 0/2] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt
[ subject is wrong, but is to match v1, see patch 2 for correct subject ] Fix reading dir again, this time without creating dentries and inodes. Changes since v1: https://lore.kernel.org/linux-trace-kernel/20240116114711.7e863...@gandalf.local.home Steven Rostedt (Google) (2): eventfs

[PATCH v2 1/2] eventfs: Have the inodes all for files and directories all be the same

2024-01-16 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The dentries and inodes are created in the readdir for the sole purpose of getting a consistent inode number. Linus stated that is unnecessary, and that all inodes can have the same inode number. For a virtual file system they are pretty meaningless. I

Re: [PATCH] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt
On Tue, 16 Jan 2024 10:53:36 -0800 Linus Torvalds wrote: > Let's at least *try* to move towards a better and simpler world, in other > words. OK, but I did just finish the below. I'll save it for another time if the single inode becomes an issue. I cropped it to just 31 bits, so I'm not sure

Re: [PATCH] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt
On Tue, 16 Jan 2024 10:21:49 -0800 Linus Torvalds wrote: > Here's a clue: just fix your inode numbers. > > I can think of many ways to do it. Here's a couple: > > - use a fixed inode number for all inodes. It's fine. Really. You might > confuse some programs that still do getpwd() the legacy

Re: [PATCH] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt
On Tue, 16 Jan 2024 13:12:28 -0500 Steven Rostedt wrote: > Maybe I can just use a hash to generate he inode numbers from the name? > Hopefully there will be no collisions. Then I don't need the dentry > creation at all. Maybe I could use a hash of the address of the meta data

Re: [PATCH] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt
On Tue, 16 Jan 2024 09:55:15 -0800 Linus Torvalds wrote: > [ html crud because I still don't have power or real Internet, just trying > to keep an eye on things on my phone. Mailing lists removed to avoid > bounces, please put them back in replies that don't have my horrible > formatting ] > >

[PATCH] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The original eventfs code added a wrapper around the dcache_readdir open callback and created all the dentries and inodes at open, and increment their ref count. A wrapper was added around the dcache_readdir release function to decrement all the

Re: [PATCH v11 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-01-15 Thread Steven Rostedt
On Mon, 15 Jan 2024 17:29:09 + Vincent Donnefort wrote: > > > > What needs to be done, and feel free to add this as a separate patch, > > is to have checks where snapshot is used. > > > > (All errors return -EBUSY) > > > > Before allowing mapping, check to see if: > > > > 1) the

Re: [PATCH v11 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-01-15 Thread Steven Rostedt
On Mon, 15 Jan 2024 11:09:38 -0500 Steven Rostedt wrote: > No. The ring buffer logic should not care if the user of it is swapping > the entire ring buffer or not. It only cares if parts of the ring > buffer is being swapped or not. That's not the level of scope it should > care abo

Re: [PATCH v11 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-01-15 Thread Steven Rostedt
On Mon, 15 Jan 2024 15:37:31 + Vincent Donnefort wrote: > > > @@ -5418,6 +5446,11 @@ int ring_buffer_swap_cpu(struct trace_buffer > > > *buffer_a, > > > cpu_buffer_a = buffer_a->buffers[cpu]; > > > cpu_buffer_b = buffer_b->buffers[cpu]; > > > > > > + if

Re: [PATCH v11 4/5] Documentation: tracing: Add ring-buffer mapping

2024-01-14 Thread Steven Rostedt
On Sun, 14 Jan 2024 23:26:43 +0900 Masami Hiramatsu (Google) wrote: > Hi Vincent, > > On Thu, 11 Jan 2024 16:17:11 + > Vincent Donnefort wrote: > > > It is now possible to mmap() a ring-buffer to stream its content. Add > > some documentation and a code example. > > > > Signed-off-by:

Re: [PATCH v11 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-01-12 Thread Steven Rostedt
On Fri, 12 Jan 2024 10:06:41 -0500 Steven Rostedt wrote: > I'm thinking both may be good, as the number of dropped events are not > added if there's no room to put it at the end of the ring buffer. When > there's no room, it just sets a flag that there was missed events but > does

Re: [PATCH v11 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-01-12 Thread Steven Rostedt
On Fri, 12 Jan 2024 09:13:02 + Vincent Donnefort wrote: > > > + > > > + unsigned long subbufs_touched; > > > + unsigned long subbufs_lost; > > > + unsigned long subbufs_read; > > > > Now I'm thinking we may not want this exported, as I'm not sure it's > > useful. > > touched and

Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default ownership

2024-01-12 Thread Steven Rostedt
On Fri, 12 Jan 2024 08:53:44 -0500 Steven Rostedt wrote: > > // We managed to open the directory so we have permission to list > > // directory entries in "xfs". > > fd = open("/sys/kernel/tracing/events/xfs"); > > > > // Remove ownership so w

Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default ownership

2024-01-12 Thread Steven Rostedt
On Fri, 12 Jan 2024 09:27:24 +0100 Christian Brauner wrote: > On Thu, Jan 11, 2024 at 04:53:19PM -0500, Steven Rostedt wrote: > > On Thu, 11 Jan 2024 22:01:32 +0100 > > Christian Brauner wrote: > > > > > What I'm pointing out in the current logic is that t

Re: [PATCH v11 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-01-11 Thread Steven Rostedt
On Thu, 11 Jan 2024 11:34:58 -0500 Mathieu Desnoyers wrote: > The LTTng kernel tracer has supported mmap'd buffers for nearly 15 years [1], > and has a lot of similarities with this patch series. > > LTTng has the notion of "subbuffer id" to allow atomically exchanging a > "reader" extra

Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default ownership

2024-01-11 Thread Steven Rostedt
On Thu, 11 Jan 2024 22:01:32 +0100 Christian Brauner wrote: > What I'm pointing out in the current logic is that the caller is > taxed twice: > > (1) Once when the VFS has done inode_permission(MAY_EXEC, "xfs") > (2) And again when you call lookup_one_len() in eventfs_start_creating() >

Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default ownership

2024-01-10 Thread Steven Rostedt
On Wed, 10 Jan 2024 10:52:51 -0500 Steven Rostedt wrote: > On Wed, 10 Jan 2024 08:07:46 -0500 > Steven Rostedt wrote: > > > Or are you saying that I don't need the ".permission" callback, because > > eventfs does it when it creates the inodes? But for eventfs t

Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default ownership

2024-01-10 Thread Steven Rostedt
On Wed, 10 Jan 2024 10:52:51 -0500 Steven Rostedt wrote: > I'll apply this patch too, as it appears to work with this code. I meant "appears to work without this code". -- Steve

Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default ownership

2024-01-10 Thread Steven Rostedt
On Wed, 10 Jan 2024 08:07:46 -0500 Steven Rostedt wrote: > Or are you saying that I don't need the ".permission" callback, because > eventfs does it when it creates the inodes? But for eventfs to know what > the permissions changes are, it uses .getattr and .setattr. OK, if

Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default ownership

2024-01-10 Thread Steven Rostedt
On Wed, 10 Jan 2024 12:45:36 +0100 Christian Brauner wrote: > So say you do: > > mkdir /sys/kernel/tracing/instances/foo > > After this has returned we know everything we need to know about the new > tracefs instance including the ownership and the mode of all inodes in >

[PATCH v3] ring-buffer: Have mmapped ring buffer keep track of missed events

2024-01-09 Thread Steven Rostedt
From: "Steven Rostedt (Google)" While testing libtracefs on the mmapped ring buffer, the test that checks if missed events are accounted for failed when using the mapped buffer. This is because the mapped page does not update the missed events that were dropped because the writer

Re: [PATCH v10 1/2] ring-buffer: Introducing ring-buffer mapping functions

2024-01-09 Thread Steven Rostedt
On Wed, 10 Jan 2024 08:42:05 +0900 Masami Hiramatsu (Google) wrote: > On Tue, 9 Jan 2024 15:13:51 + > Vincent Donnefort wrote: > > > > > @@ -388,6 +389,7 @@ struct rb_irq_work { > > > > boolwaiters_pending; > > > > bool

<    1   2   3   4   5   6   7   8   9   10   >