Re: [RFC PATCH 03/24] erofs: add Errno in Rust

[Gao Xiang]

On 2024/9/26 05:45, Ariel Miculas wrote:
> On 24/09/26 12:35, Gao Xiang wrote:
> > Hi Ariel,
> >
> > On 2024/9/25 23:48, Ariel Miculas wrote:

...

> > And there are all thoughts for reference [2][3][4][5]:
> > [2] https://github.com/project-machine/puzzlefs/issues/114#issuecomment-2369872133
> > [3] https://github.com/opencontainers/image-spec/issues/1190#issuecomment-2138572683
> > [4] https://lore.kernel.org/linux-fsdevel/b9358e7c-8615-1b12-e35d-aae59bf6a467@xxxxxxxxxxxxxxxxx/
> > [5] https://lore.kernel.org/linux-fsdevel/20230609-nachrangig-handwagen-375405d3b9f1@brauner/
> >
> > Here still, I do really want to collaborate with you on your
> > reasonable use cases.  But if you really want to do your upstream
> > attempt without even any comparsion, please go ahead because I
> > believe I can only express my own opinion, but I really don't
> > decide if your work is acceptable for the kernel.
>
> Thanks for your thoughts on PuzzleFS, I would really like if we could
> centralize the discussions on the latest patch series I sent to the
> mailing lists back in May [1]. The reason I say this is because looking
> at that thread, it seems there is no feedback for PuzzleFS. The feedback
> exists, it's just scattered throughout different mediums. On top of
> this, I would also like to engage in the discussions with Dave Chinner,
> so I can better understand the limitations of PuzzleFS and the reasons
> for which it might be rejected in the Linux Kernel. I do appreciate your
> feedback and I need to take my time to respond to the technical issues
> that you brought up in the github issue.

In short, I really want to avoid open arbitary number files in the
page fault path regardless of the performance concerns, because
even there are many cases that mmap_lock is dropped, but IMHO there
is still cases that mmap_lock will be taken.

IOWs, I think it's controversal for a kernel fs to open random file
in the page fault context under mmap_lock in the begining.
Otherwise, it's pretty straight-forward to add some similiar feature
to EROFS.

> However, even if it's not upstream, PuzzleFS does use the latest Rust
> filesystem abstractions and thus it stands as an example of how to use
> them. And this thread is not about PuzzleFS, but about the Rust
> filesystem abstractions and how one might start to use them. That's
> where I offered to help, since I already went through the process of
> having to use them.
>
> [1] https://lore.kernel.org/all/20240516190345.957477-1-amiculas@xxxxxxxxx/
>
> > > I'm happy to help you if you decide to go down this route.
> >
> > Again, the current VFS abstraction is totally incomplete and broken
> > [6].
>
> If they're incomplete, we can work together to implement the missing
> functionalities. Furthermore, we can work to fix the broken stuff. I
> don't think these are good reasons to completely ignore the work that's
> already been done on this topic.

I've said, we don't miss any Rust VFS abstraction work, as long as
some work lands in the Linux kernel, we will switch to use them.

The reason we don't do that is again
 - I don't have time to work on this because my life is still limited
   for RFL in any way at least this year; I don't know if Yiyang has
   time to work on a complete ext2 and a Rust VFS abstraction.

 - We just would like to _use Rust_ for the core EROFS logic, instead
   of touching any VFS stuff.  I'm not sure why it's called "completely
   ignore the VFS abstraction", because there is absolutely no
   relationship between these two things.  Why we need to mix them up?

> By the way, what is it that's actually broken? You've linked to an LWN
> article [2] (or at least I think your 6th link was supposed to link to
> "Rust for filesystems" instead of the "Committing to Rust in the kernel"
> one), but I'm interested in the specifics. What exactly doesn't work as
> expected from the filesystem abstractions?

For example, with my current Rust skill, I'm not sure why
fill_super for "T::SUPER_TYPE, sb::Type::BlockDev" must use
"new_sb.bdev().inode().mapper()".

It's unnecessary for a bdev-based fs to use bdev inode page
cache to read metadata;

Also it's unnecessary for a const fs type to be
sb::Type::BlockDev or sb::Type::Independent as

/// Determines how superblocks for this file system type are keyed.
+    const SUPER_TYPE: sb::Type = sb::Type::Independent;

because at least for the current EROFS use cases, we will
decide to use get_tree_bdev() or get_tree_nodev() according
to the mount source or mount options.

> [2] https://lwn.net/Articles/978738/
>
> > I believe it should be driven by a full-featured read-write fs [7]
> > (even like a simple minix fs in pre-Linux 1.0 era) and EROFS will
>
> I do find it weird that you want a full-featured read-write fs
> implemented in Rust, when erofs is a read-only filesystem.

I'm not sure why it's weird from the sane Rust VFS abstraction
perspective.

> > use Rust in "fs/erofs" as the experiment, but I will definitely
> > polish the Rust version until it looks good before upstreaming.
>
> I honestly don't see how it would look good if they're not using the
> existing filesystem abstractions. And I'm not convinced that Rust in the
> kernel would be useful in any way without the many subsystem
> abstractions which were implemented by the Rust for Linux team for the
> past few years.

So let's see the next version.

Thanks,
Gao Xiang

> Cheers,
> Ariel