Received-SPF: pass (google.com: domain of linux-kernel+bounces-42260-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240126150209.367ff402@gandalf.local.home> <CAHk-=wgZEHwFRgp2Q8_-OtpCtobbuFPBmPTZ68qN3MitU-ub=Q@mail.gmail.com>
 <20240126162626.31d90da9@gandalf.local.home> <CAHk-=wj8WygQNgoHerp-aKyCwFxHeyKMguQszVKyJfi-=Yfadw@mail.gmail.com>
 <CAHk-=whNfNti-mn6vhL-v-WZnn0i7ZAbwSf_wNULJeyanhPOgg@mail.gmail.com>
 <CAHk-=wj+DsZZ=2iTUkJ-Nojs9fjYMvPs1NuoM3yK7aTDtJfPYQ@mail.gmail.com>
 <20240128175111.69f8b973@rorschach.local.home> <CAHk-=wjHc48QSGWtgBekej7F+Ln3b0j1tStcqyEf3S-Pj_MHHw@mail.gmail.com>
 <20240128185943.6920388b@rorschach.local.home> <20240128192108.6875ecf4@rorschach.local.home>
 <CAHk-=wg7tML8L+27j=7fh8Etk4Wvo0Ay3mS5U7JOTEGxjy1viA@mail.gmail.com> <20240128210938.436fc3b4@rorschach.local.home>
In-Reply-To: <20240128210938.436fc3b4@rorschach.local.home>
From: Amir Goldstein <amir73il@gmail.com>
Date: Mon, 29 Jan 2024 08:44:29 +0200
Message-ID: <CAOQ4uxikMN9EapL5+6jtMn5Ahe4h7wGZOgEsdsjy87v72FxJDw@mail.gmail.com>
Subject: Re: [PATCH] eventfs: Have inodes have unique inode numbers
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>, Masami Hiramatsu <mhiramat@kernel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, LKML <linux-kernel@vger.kernel.org>, 
	Linux Trace Devel <linux-trace-devel@vger.kernel.org>, Christian Brauner <brauner@kernel.org>, 
	Ajay Kaher <ajay.kaher@broadcom.com>, Geert Uytterhoeven <geert@linux-m68k.org>, 
	linux-fsdevel <linux-fsdevel@vger.kernel.org>, Greg KH <gregkh@linuxfoundation.org>, 
	Al Viro <viro@zeniv.linux.org.uk>
Content-Type: text/plain; charset="UTF-8"

> >
> > You need to deal with the realities of having made a filesystem. And
> > one of those realities is that you don't control the dentries, and you
> > can't randomly cache dentry state and then do things behind the VFS
> > layer's back.
>
> I'm not. I'm trying to let VFS know a directory is deleted. Because
> when you delete a kprobe, the directory that has the control files for
> that kprobe (like enabling it) go away too. I have to let VFS know that
> the directory is deleted, just like procfs has to tell it when a
> directory for a process id is no more.
>
> You don't kill tasks with: rmdir /proc/1234
>
> And you don't delete kprobes with: rmdir events/kprobe/sched
>
> >
> > So remove that broken function. Really.  You did a filesystem, and
> > that means that you had better play by the VFS rules.
> >
> > End of discussion.
>
> And I do it just like debugfs when it deletes files outside of VFS or
> procfs, and pretty much most virtual file systems.
>

I think it is better if we used the term "pseudo" file systems, because
to me VFS already stands for "virtual file system".

> >
> > Now, you can then make your own "read" and "lookup" etc functions say
> > "if the backing store data has been marked dead, I'll not do this".
> > That's *YOUR* data structures, and that's your choice.
> >
> > But you need to STOP thinking you can play games with dentries.  And
> > you need to stop making up BS arguments for why  you should be able
> > to.
> >
> > So if you are thinking of a "Here's how to do a virtual filesystem"
> > talk, I would suggest you start with one single word: "Don't".
> >
> > I'm convinced that we have made it much too easy to do a half-arsed
> > virtual filesystem. And eventfs is way beyond half-arsed.
> >
> > It's now gone from half-arsed to "I told you how to do this right, and
> > you are still arguing". That makes it full-arsed.
> >
> > So just stop the arsing around, and just get rid of those _broken_ dentry games.
>
> Sorry, but you didn't prove your point. The example you gave me is
> already covered. Please tell me when a kprobe goes away, how do I let
> VFS know? Currently the tracing code (like kprobes and synthetic
> events) calls eventfs_remove_dir() with just a reference to that ei
> eventfs_inode structure. I currently use the ei->dentry to tell VFS
> "this directory is being deleted". What other means do I have to
> accomplish the same thing?
>

Would kernfs_node_dentry() have been useful in that case?
Just looking at kernfs_node and eventfs_inode gives a very strong
smell of reinventing.

Note that the fact that eventfs has no rename makes the whole dentry
business a lot less complicated than it is in the general VFS case -
IIUC, an eventfs path either exists or it does not exist, but if it exists,
it conceptually always refers to the same underlying object (kprobe).

I am not claiming that kernfs can do everything that eventfs needs  -
I don't know, because I did not look into it.

But the concept of detaching the pseudo filesystem backing objects
from the vfs objects was already invented once and Greg has also
said the same thing.

My *feeling* at this point is that the best course of action is to use
kernfs and to improve kernfs to meet eventfs needs, if anything needs
improving at all.

IMO, the length and content of this correspondence in itself
is proof that virtual file systems should use an abstraction that
is much less flexible than the VFS.

Think of it this way - kernefs and VFS are both under-documented,
but the chances of getting kernfs well documented are far better
than ever being able to document all the subtleties of VFS for mortals.

IOW, I would be happy if instead of the LSFMM topic
"Making pseudo file systems inodes/dentries more like normal file systems"
We would be talking about
"Best practices for writing a pseudo filesystem" and/or
"Missing kernfs functionality for writing pseudo filesystems"

Thanks,
Amir.