linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gao Xiang <hsiangkao@linux.alibaba.com>
To: Benno Lossin <benno.lossin@proton.me>,
	Gary Guo <gary@garyguo.net>, Yiyang Wu <toolmanp@tlmp.cc>
Cc: linux-erofs@lists.ozlabs.org, rust-for-linux@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [RFC PATCH 03/24] erofs: add Errno in Rust
Date: Thu, 19 Sep 2024 23:13:41 +0800	[thread overview]
Message-ID: <9bbbac63-c05f-4f7b-91c2-141a93783cd3@linux.alibaba.com> (raw)
In-Reply-To: <ac871d1e-9e4e-4d1b-82be-7ae87b78d33e@proton.me>

Hi Benno,

On 2024/9/19 21:45, Benno Lossin wrote:
> 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.

I'm not quite sure about your point honestly.  In my opinion, there
is nothing different to use Rust _within a filesystem_ or _within a
driver_ or _within a Linux subsystem_ as long as all negotiated APIs
are audited.

Otherwise, it means Rust will never be used to write Linux core parts
such as MM, VFS or block layer. Does this point make sense? At least,
Rust needs to get along with the existing C code (in an audited way)
rather than refuse C code.

My personal idea about Rust: I think Rust is just another _language
tool_ for the Linux kernel which could save us time and make the
kernel development better.

Or I wonder why not writing a complete new Rust stuff instead rather
than living in the C world?

> 
>> 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?

It's not written by me, Yiyang is still an undergraduate tudent.
It's his research project and I don't think it's his responsibility
to make an upstreamable VFS abstraction.

> 
> I have quickly glanced over the patchset and the abstractions seem
> rather immature, not general enough for other filesystems to also take

I don't have enough time to take a full look of this patchset too
due to other ongoing work for now (Rust EROFS is not quite a high
priority stuff for me).

And that's why it's called "RFC PATCH".

> advantage of them. They also miss safety documentation and are in

I don't think it needs to be general enough, since we'd like to use
the new Rust language tool within a subsystem.

So why it needs to take care of other filesystems? Again, I'm not
working on a full VFS abstriction.

Yes, this patchset is not perfect.  But I've asked Yiyang to isolate
all VFS structures as much as possible, but it seems that it still
touches something.

> general poorly documented.

Okay, I think it can be improved then if you give more detailed hints.

> 
> 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.

I think Rust is just a tool, if you could have extra time to review
our work, that would be wonderful!  Many thanks then.

However, if you don't have time to review, IMHO, Rust is just a tool,
I think each subsystem can choose to use Rust in their codebase, or
I'm not sure what's your real point is?

> 
>> 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).

Leaving aside everything else, an incomplete Rust read-only VFS
abstraction itself is just an unsafe stuff.

> 
>> (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.

I've expressed my viewpoint.

> 
>> 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.

Without a full proper VFS abstraction, it's just broken and
needs to be refactored.  And that will be painful to all
users then.

=======
In the end,

Other thoughts, comments are helpful here since I wonder how "Rust
-for-Linux" works in the long term, and decide whether I will work
on Kernel Rust or not at least in the short term.

Thanks,
Gao Xiang

> 
> ---
> Cheers,
> Benno
> 


  reply	other threads:[~2024-09-19 15:13 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-16 13:56 [RFC PATCH 00/24] erofs: introduce Rust implementation Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 01/24] erofs: lift up erofs_fill_inode to global Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 02/24] erofs: add superblock data structure in Rust Yiyang Wu
2024-09-16 17:55   ` Greg KH
2024-09-17  0:18     ` Gao Xiang
2024-09-17  5:34       ` Greg KH
2024-09-17  5:45         ` Gao Xiang
2024-09-17  5:27     ` Yiyang Wu
2024-09-17  5:39     ` Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 03/24] erofs: add Errno " Yiyang Wu
2024-09-16 17:51   ` Greg KH
2024-09-16 23:45     ` Gao Xiang
2024-09-20  2:49     ` [PATCH RESEND 0/1] rust: introduce declare_err! autogeneration Yiyang Wu
2024-09-20  2:49       ` [PATCH RESEND 1/1] rust: error: auto-generate error declarations Yiyang Wu
2024-09-20  2:57     ` [RFC PATCH 03/24] erofs: add Errno in Rust Yiyang Wu
2024-09-16 20:01   ` Gary Guo
2024-09-16 23:58     ` Gao Xiang
2024-09-19 13:45       ` Benno Lossin
2024-09-19 15:13         ` Gao Xiang [this message]
2024-09-19 19:36           ` Benno Lossin
2024-09-20  0:49             ` Gao Xiang
2024-09-21  8:37               ` Greg Kroah-Hartman
2024-09-21  9:29                 ` Gao Xiang
2024-09-25 15:48             ` Ariel Miculas
2024-09-25 16:35               ` Gao Xiang
2024-09-25 21:45                 ` Ariel Miculas
2024-09-26  0:40                   ` Gao Xiang
2024-09-26  1:04                     ` Gao Xiang
2024-09-26  8:10                       ` Ariel Miculas
2024-09-26  8:25                         ` Gao Xiang
2024-09-26  9:51                           ` Ariel Miculas
2024-09-26 10:46                             ` Gao Xiang
2024-09-26 11:01                               ` Ariel Miculas
2024-09-26 11:05                                 ` Gao Xiang
2024-09-26 11:23                                 ` Gao Xiang
2024-09-26 12:50                                   ` Ariel Miculas
2024-09-27  2:18                                     ` Gao Xiang
2024-09-26  8:48                         ` Gao Xiang
2024-09-16 13:56 ` [RFC PATCH 04/24] erofs: add xattrs data structure " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 05/24] erofs: add inode " Yiyang Wu
2024-09-18 13:04   ` [External Mail][RFC " Huang Jianan
2024-09-16 13:56 ` [RFC PATCH 06/24] erofs: add alloc_helper " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 07/24] erofs: add data abstraction " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 08/24] erofs: add device data structure " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 09/24] erofs: add continuous iterators " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 10/24] erofs: add device_infos implementation " Yiyang Wu
2024-09-21  9:44   ` Jianan Huang
2024-09-16 13:56 ` [RFC PATCH 11/24] erofs: add map data structure " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 12/24] erofs: add directory entry " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 13/24] erofs: add runtime filesystem and inode " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 14/24] erofs: add block mapping capability " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 15/24] erofs: add iter methods in filesystem " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 16/24] erofs: implement dir and inode operations " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 17/24] erofs: introduce Rust SBI to C Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 18/24] erofs: introduce iget alternative " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 19/24] erofs: introduce namei " Yiyang Wu
2024-09-16 17:08   ` Al Viro
2024-09-17  6:48     ` Yiyang Wu
2024-09-17  7:14       ` Gao Xiang
2024-09-17  7:31         ` Al Viro
2024-09-17  7:44           ` Al Viro
2024-09-17  8:08             ` Gao Xiang
2024-09-17 22:22             ` Al Viro
2024-09-17  8:06           ` Gao Xiang
2024-09-16 13:56 ` [RFC PATCH 20/24] erofs: introduce readdir " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 21/24] erofs: introduce erofs_map_blocks " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 22/24] erofs: add skippable iters in Rust Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 23/24] erofs: implement xattrs operations " Yiyang Wu
2024-09-16 13:56 ` [RFC PATCH 24/24] erofs: introduce xattrs replacement to C Yiyang Wu
  -- strict thread matches above, loose matches on Subject: below --
2024-09-16 13:55 [RFC PATCH 00/24] erofs: introduce Rust implementation Yiyang Wu
2024-09-16 13:55 ` [RFC PATCH 03/24] erofs: add Errno in Rust Yiyang Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9bbbac63-c05f-4f7b-91c2-141a93783cd3@linux.alibaba.com \
    --to=hsiangkao@linux.alibaba.com \
    --cc=benno.lossin@proton.me \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=toolmanp@tlmp.cc \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).