[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240130120634.2f0ecd0a@gandalf.local.home>
Date: Tue, 30 Jan 2024 12:06:34 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: kernel test robot <oliver.sang@...el.com>, oe-lkp@...ts.linux.dev,
lkp@...el.com, linux-kernel@...r.kernel.org, Masami Hiramatsu
<mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>, Mathieu
Desnoyers <mathieu.desnoyers@...icios.com>, Christian Brauner
<brauner@...nel.org>, Al Viro <viro@...iv.linux.org.uk>, Ajay Kaher
<ajay.kaher@...adcom.com>, linux-trace-kernel@...r.kernel.org
Subject: Re: [linus:master] [eventfs] 852e46e239:
BUG:unable_to_handle_page_fault_for_address
On Tue, 30 Jan 2024 08:55:51 -0800
Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> On Tue, 30 Jan 2024 at 08:49, Steven Rostedt <rostedt@...dmis.org> 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
> > deleted by walking the ei->parent until I find the events inode that has
> > a dentry. I then use that to do a lookup walking back down to the
> > eventfs_inode I want to delete. This gives me the dentry that I can call
> > d_invalidate() on.
>
> Yes, that works.
>
> However, I have a patch that is *much* smaller and simpler, and
> doesn't need that walk.
>
> The VFS layer already has a good interface for "should I still use
> this dentry", which is needed for various network filesystems etc that
> want to time out caches (or check explicitly whether the file still
> exists etc): it's the dentry d_revalidate() check.
>
> Let me just reboot into it to test that I got all the cases.
>
> It makes the code even more obvious, and avoids all the complexity.
I actually had this before, but it wasn't working (likely to something else
that wasn't working or I did it wrong) so I reverted it.
-- Steve
diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 49d4630d5d70..9867b39ae24c 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -451,6 +451,13 @@ lookup_file_dentry(struct dentry *dentry,
return dentry;
}
+int eventfs_revalidate(struct dentry *dentry, unsigned int flags)
+{
+ struct eventfs_inode *ei = dentry->d_fsdata;
+
+ return ei && !ei->is_freed;
+}
+
/**
* eventfs_root_lookup - lookup routine to create file/dir
* @dir: in which a lookup is being done
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index e1b172c0e091..0395459d919e 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -392,8 +392,24 @@ static void tracefs_dentry_iput(struct dentry *dentry, struct inode *inode)
iput(inode);
}
+static int tracefs_revalidate(struct dentry *dentry, unsigned int flags)
+{
+ struct inode *inode = dentry->d_inode;
+ struct tracefs_inode *ti;
+
+ if (!dentry || !inode)
+ return 0;
+
+ ti = get_tracefs(inode);
+ if (!ti || !(ti->flags & TRACEFS_EVENT_INODE))
+ return 1;
+
+ return eventfs_revalidate(dentry, flags);
+}
+
static const struct dentry_operations tracefs_dentry_operations = {
- .d_iput = tracefs_dentry_iput,
+ .d_iput = tracefs_dentry_iput,
+ .d_revalidate = tracefs_revalidate,
};
static int trace_fill_super(struct super_block *sb, void *data, int silent)
diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h
index 2af78fd95c93..a1024202c4e5 100644
--- a/fs/tracefs/internal.h
+++ b/fs/tracefs/internal.h
@@ -80,5 +80,6 @@ struct dentry *eventfs_start_creating(const char *name, struct dentry *parent);
struct dentry *eventfs_failed_creating(struct dentry *dentry);
struct dentry *eventfs_end_creating(struct dentry *dentry);
void eventfs_set_ei_status_free(struct tracefs_inode *ti, struct dentry *dentry);
+int eventfs_revalidate(struct dentry *dentry, unsigned int flags);
#endif /* _TRACEFS_INTERNAL_H */
Powered by blists - more mailing lists