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

[Benno Lossin <benno.lossin@proton.me>]

Hi,

Thanks for the patch series. I think it's great that you want to use
Rust for this filesystem.

On 17.09.24 01:58, Gao Xiang wrote:
> On 2024/9/17 04:01, Gary Guo wrote:
>> Also, it seems that you're building abstractions into EROFS directly
>> without building a generic abstraction. We have been avoiding that. If
>> there's an abstraction that you need and missing, please add that
>> abstraction. In fact, there're a bunch of people trying to add FS
> 
> No, I'd like to try to replace some EROFS C logic first to Rust (by
> using EROFS C API interfaces) and try if Rust is really useful for
> a real in-tree filesystem.  If Rust can improve EROFS security or
> performance (although I'm sceptical on performance), As an EROFS
> maintainer, I'm totally fine to accept EROFS Rust logic landed to
> help the whole filesystem better.

As Gary already said, we have been using a different approach and it has
served us well. Your approach of calling directly into C from the driver
can be used to create a proof of concept, but in our opinion it is not
something that should be put into mainline. That is because calling C
from Rust is rather complicated due to the many nuanced features that
Rust provides (for example the safety requirements of references).
Therefore moving the dangerous parts into a central location is crucial
for making use of all of Rust's advantages inside of your code.

> For Rust VFS abstraction, that is a different and indepenent story,
> Yiyang don't have any bandwidth on this due to his limited time.

This seems a bit weird, you have the bandwidth to write your own
abstractions, but not use the stuff that has already been developed?

I have quickly glanced over the patchset and the abstractions seem
rather immature, not general enough for other filesystems to also take
advantage of them. They also miss safety documentation and are in
general poorly documented.

Additionally, all of the code that I saw is put into the `fs/erofs` and
`rust/erofs_sys` directories. That way people can't directly benefit
from your code, put your general abstractions into the kernel crate.
Soon we will be split the kernel crate, I could imagine that we end up
with an `fs` crate, when that happens, we would put those abstractions
there.

As I don't have the bandwidth to review two different sets of filesystem
abstractions, I can only provide you with feedback if you use the
existing abstractions.

> And I _also_ don't think an incomplete ROFS VFS Rust abstraction
> is useful to Linux community

IIRC Wedson created ROFS VFS abstractions before going for the full
filesystem. So it would definitely be useful for other read-only
filesystems (as well as filesystems that also allow writing, since last
time I checked, they often also support reading).

> (because IMO for generic interface
> design, we need a global vision for all filesystems instead of
> just ROFSes.  No existing user is not an excuse for an incomplete
> abstraction.)

Yes we need a global vision, but if you would use the existing
abstractions, then you would participate in this global vision.

Sorry for repeating this point so many times, but it is *really*
important that we don't have multiple abstractions for the same thing.

> If a reasonble Rust VFS abstraction landed, I think we will switch
> to use that, but as I said, they are completely two stories.

For them to land, there has to be some kind of user. For example, a rust
reference driver, or a new filesystem. For example this one.

---
Cheers,
Benno