| Message ID | 20240916135634.98554-4-toolmanp@tlmp.cc (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | erofs: introduce Rust implementation | expand |
On Mon, Sep 16, 2024 at 09:56:13PM +0800, Yiyang Wu wrote: > Introduce Errno to Rust side code. Note that in current Rust For Linux, > Errnos are implemented as core::ffi::c_uint unit structs. > However, EUCLEAN, a.k.a EFSCORRUPTED is missing from error crate. > > Since the errno_base hasn't changed for over 13 years, > This patch merely serves as a temporary workaround for the missing > errno in the Rust For Linux. Why not just add the missing errno to the core rust code instead? No need to define a whole new one for this. thanks, greg k-h
On Mon, 16 Sep 2024 21:56:13 +0800 Yiyang Wu <toolmanp@tlmp.cc> wrote: > Introduce Errno to Rust side code. Note that in current Rust For Linux, > Errnos are implemented as core::ffi::c_uint unit structs. > However, EUCLEAN, a.k.a EFSCORRUPTED is missing from error crate. > > Since the errno_base hasn't changed for over 13 years, > This patch merely serves as a temporary workaround for the missing > errno in the Rust For Linux. > > Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc> As Greg said, please add missing errno that you need to kernel crate instead. 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 support, please coordinate instead of rolling your own. You also have been referencing `kernel::bindings::` directly in various places in the patch series. The module is marked as `#[doc(hidden)]` for a reason -- it's not supposed to referenced directly. It's only exposed so that macros can reference them. In fact, we have a policy that direct reference to raw bindings are not allowed from drivers. There're a few issues with this patch itself that I pointed out below, although as already said this would require big changes so most points are probably moot anyway. Thanks, Gary > --- > fs/erofs/rust/erofs_sys.rs | 6 + > fs/erofs/rust/erofs_sys/errnos.rs | 191 ++++++++++++++++++++++++++++++ > 2 files changed, 197 insertions(+) > create mode 100644 fs/erofs/rust/erofs_sys/errnos.rs > > diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs > index 0f1400175fc2..2bd1381da5ab 100644 > --- a/fs/erofs/rust/erofs_sys.rs > +++ b/fs/erofs/rust/erofs_sys.rs > @@ -19,4 +19,10 @@ > pub(crate) type Nid = u64; > /// Erofs Super Offset to read the ondisk superblock > pub(crate) const EROFS_SUPER_OFFSET: Off = 1024; > +/// PosixResult as a type alias to kernel::error::Result > +/// to avoid naming conflicts. > +pub(crate) type PosixResult<T> = Result<T, Errno>; > + > +pub(crate) mod errnos; > pub(crate) mod superblock; > +pub(crate) use errnos::Errno; > diff --git a/fs/erofs/rust/erofs_sys/errnos.rs b/fs/erofs/rust/erofs_sys/errnos.rs > new file mode 100644 > index 000000000000..40e5cdbcb353 > --- /dev/null > +++ b/fs/erofs/rust/erofs_sys/errnos.rs > @@ -0,0 +1,191 @@ > +// Copyright 2024 Yiyang Wu > +// SPDX-License-Identifier: MIT or GPL-2.0-or-later > + > +#[repr(i32)] > +#[non_exhaustive] > +#[allow(clippy::upper_case_acronyms)] > +#[derive(Debug, Copy, Clone, PartialEq)] > +pub(crate) enum Errno { > + NONE = 0, Why is NONE an error? No "error: operation completed successfully" please. > + EPERM, > + ENOENT, > + ESRCH, > + EINTR, > + EIO, > + ENXIO, > + E2BIG, > + ENOEXEC, > + EBADF, > + ECHILD, > + EAGAIN, > + ENOMEM, > + EACCES, > + EFAULT, > + ENOTBLK, > + EBUSY, > + EEXIST, > + EXDEV, > + ENODEV, > + ENOTDIR, > + EISDIR, > + EINVAL, > + ENFILE, > + EMFILE, > + ENOTTY, > + ETXTBSY, > + EFBIG, > + ENOSPC, > + ESPIPE, > + EROFS, > + EMLINK, > + EPIPE, > + EDOM, > + ERANGE, > + EDEADLK, > + ENAMETOOLONG, > + ENOLCK, > + ENOSYS, > + ENOTEMPTY, > + ELOOP, > + ENOMSG = 42, This looks very fragile way to maintain an enum. > + EIDRM, > + ECHRNG, > + EL2NSYNC, > + EL3HLT, > + EL3RST, > + ELNRNG, > + EUNATCH, > + ENOCSI, > + EL2HLT, > + EBADE, > + EBADR, > + EXFULL, > + ENOANO, > + EBADRQC, > + EBADSLT, > + EBFONT = 59, > + ENOSTR, > + ENODATA, > + ETIME, > + ENOSR, > + ENONET, > + ENOPKG, > + EREMOTE, > + ENOLINK, > + EADV, > + ESRMNT, > + ECOMM, > + EPROTO, > + EMULTIHOP, > + EDOTDOT, > + EBADMSG, > + EOVERFLOW, > + ENOTUNIQ, > + EBADFD, > + EREMCHG, > + ELIBACC, > + ELIBBAD, > + ELIBSCN, > + ELIBMAX, > + ELIBEXEC, > + EILSEQ, > + ERESTART, > + ESTRPIPE, > + EUSERS, > + ENOTSOCK, > + EDESTADDRREQ, > + EMSGSIZE, > + EPROTOTYPE, > + ENOPROTOOPT, > + EPROTONOSUPPORT, > + ESOCKTNOSUPPORT, > + EOPNOTSUPP, > + EPFNOSUPPORT, > + EAFNOSUPPORT, > + EADDRINUSE, > + EADDRNOTAVAIL, > + ENETDOWN, > + ENETUNREACH, > + ENETRESET, > + ECONNABORTED, > + ECONNRESET, > + ENOBUFS, > + EISCONN, > + ENOTCONN, > + ESHUTDOWN, > + ETOOMANYREFS, > + ETIMEDOUT, > + ECONNREFUSED, > + EHOSTDOWN, > + EHOSTUNREACH, > + EALREADY, > + EINPROGRESS, > + ESTALE, > + EUCLEAN, > + ENOTNAM, > + ENAVAIL, > + EISNAM, > + EREMOTEIO, > + EDQUOT, > + ENOMEDIUM, > + EMEDIUMTYPE, > + ECANCELED, > + ENOKEY, > + EKEYEXPIRED, > + EKEYREVOKED, > + EKEYREJECTED, > + EOWNERDEAD, > + ENOTRECOVERABLE, > + ERFKILL, > + EHWPOISON, > + EUNKNOWN, > +} > + > +impl From<i32> for Errno { > + fn from(value: i32) -> Self { > + if (-value) <= 0 || (-value) > Errno::EUNKNOWN as i32 { > + Errno::EUNKNOWN > + } else { > + // Safety: The value is guaranteed to be a valid errno and the memory > + // layout is the same for both types. > + unsafe { core::mem::transmute(value) } This is just unsound. As evident from the fact that you need to manually specify a few constants, the errno enum doesn't cover all values from 1 to EUNKNOWN. > + } > + } > +} > + > +impl From<Errno> for i32 { > + fn from(value: Errno) -> Self { > + -(value as i32) > + } > +} > + > +/// Replacement for ERR_PTR in Linux Kernel. > +impl From<Errno> for *const core::ffi::c_void { > + fn from(value: Errno) -> Self { > + (-(value as core::ffi::c_long)) as *const core::ffi::c_void > + } > +} > + > +impl From<Errno> for *mut core::ffi::c_void { > + fn from(value: Errno) -> Self { > + (-(value as core::ffi::c_long)) as *mut core::ffi::c_void > + } > +} > + > +/// Replacement for PTR_ERR in Linux Kernel. > +impl From<*const core::ffi::c_void> for Errno { > + fn from(value: *const core::ffi::c_void) -> Self { > + (-(value as i32)).into() > + } > +} > + > +impl From<*mut core::ffi::c_void> for Errno { > + fn from(value: *mut core::ffi::c_void) -> Self { > + (-(value as i32)).into() > + } > +} > +/// Replacement for IS_ERR in Linux Kernel. > +#[inline(always)] > +pub(crate) fn is_value_err(value: *const core::ffi::c_void) -> bool { > + (value as core::ffi::c_ulong) >= (-4095 as core::ffi::c_long) as core::ffi::c_ulong > +}
Hi Greg, On 2024/9/17 01:51, Greg KH wrote: > On Mon, Sep 16, 2024 at 09:56:13PM +0800, Yiyang Wu wrote: >> Introduce Errno to Rust side code. Note that in current Rust For Linux, >> Errnos are implemented as core::ffi::c_uint unit structs. >> However, EUCLEAN, a.k.a EFSCORRUPTED is missing from error crate. >> >> Since the errno_base hasn't changed for over 13 years, >> This patch merely serves as a temporary workaround for the missing >> errno in the Rust For Linux. > > Why not just add the missing errno to the core rust code instead? No > need to define a whole new one for this. I've discussed with Yiyang about this last week. I also tend to avoid our own errno. The main reason is that Rust errno misses EUCLEAN error number. TBH, I don't know why not just introduces all kernel supported errnos for Rust in one shot. I guess just because no Rust user uses other errno? But for errno cases, I think it's odd for users to add their own errno. Thanks, Gao Xiang > > thanks, > > greg k-h
Hi Gary, On 2024/9/17 04:01, Gary Guo wrote: > On Mon, 16 Sep 2024 21:56:13 +0800 > Yiyang Wu <toolmanp@tlmp.cc> wrote: > >> Introduce Errno to Rust side code. Note that in current Rust For Linux, >> Errnos are implemented as core::ffi::c_uint unit structs. >> However, EUCLEAN, a.k.a EFSCORRUPTED is missing from error crate. >> >> Since the errno_base hasn't changed for over 13 years, >> This patch merely serves as a temporary workaround for the missing >> errno in the Rust For Linux. >> >> Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc> > > As Greg said, please add missing errno that you need to kernel crate > instead. I've answered Greg about this in another email. > > 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. For Rust VFS abstraction, that is a different and indepenent story, Yiyang don't have any bandwidth on this due to his limited time. And I _also_ don't think an incomplete ROFS VFS Rust abstraction is useful to Linux community (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.) If a reasonble Rust VFS abstraction landed, I think we will switch to use that, but as I said, they are completely two stories. > support, please coordinate instead of rolling your own. > > You also have been referencing `kernel::bindings::` directly in various > places in the patch series. The module is marked as `#[doc(hidden)]` > for a reason -- it's not supposed to referenced directly. It's only > exposed so that macros can reference them. In fact, we have a policy > that direct reference to raw bindings are not allowed from drivers. This patch can be avoided if EUCLEAN is added to errno. Thanks, Gao Xiang
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
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 >
On 19.09.24 17:13, Gao Xiang wrote: > 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. To us there is a big difference: If a lot of functions in an API are `unsafe` without being inherent from the problem that it solves, then it's a bad API. > 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. I am neither requiring you to write solely safe code, nor am I banning interacting with the C side. What we mean when we talk about abstractions is that we want to minimize the Rust code that directly interfaces with C. Rust-to-Rust interfaces can be a lot safer and are easier to implement correctly. > 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. Yes, but we do have conventions, rules and guidelines for writing such code. C code also has them. If you want/need to break them, there should be a good reason to do so. I don't see one in this instance. > Or I wonder why not writing a complete new Rust stuff instead rather > than living in the C world? There are projects that do that yes. But Rust-for-Linux is about bringing Rust to the kernel and part of that is coming up with good conventions and rules. >>> 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. That is fair, but he wouldn't have to start from scratch, Wedsons abstractions were good enough for him to write a Rust version of ext2. In addition, tarfs and puzzlefs also use those bindings. To me it sounds as if you have not taken the time to try to make it work with the existing abstractions. Have you tried reaching out to Ariel? He is working on puzzlefs and might have some insight to give you. Sadly Wedson has left the project, so someone will have to pick up his work. I hope that you understand that we can't have two abstractions for the same C API. It confuses people which to use, some features might only be available in one version and others only in the other. It would be a total mess. It's just like the rule for no duplicated drivers that you have on the C side. People (mostly Wedson) also have put in a lot of work into making the VFS abstractions good. Why ignore all of that? >> 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". Yeah I saw the RFC title. I just wanted to communicate early that I would not review it if it were a normal patch. In fact, I would advise against taking the patch, due to the reasons I outlined. >> 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. And that's OK, feel free to just pick the parts of the existing VFS that you need and extend as you (or your student) see fit. What you said yourself is that we need a global vision for VFS abstractions. If you only use a subset of them, then you only care about that subset, other people can extend it if they need. If everyone would roll their own abstractions without communicating, then how would we create a global vision? > 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. It would already be a big improvement to put the VFS structures into the kernel crate. Because then everyone can benefit from your work. >> 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? I don't want to prevent or discourage you from using Rust in the kernel. In fact, I can't prevent you from putting this in, since after all you are the maintainer. What I can do, is advise against not using abstractions. That has been our philosophy since very early on. They are the reason that you can write PHY drivers without any `unsafe` code whatsoever *today*. I think that is an impressive feat and our recipe for success. We even have this in our documentation: https://docs.kernel.org/rust/general-information.html#abstractions-vs-bindings My real point is that I want Rust to succeed in the kernel. I strongly believe that good abstractions (in the sense that you can do as much as possible using only safe Rust) are a crucial factor. I and others from the RfL team can help you if you (or your student) have any Rust related questions for the abstractions. Feel free to reach out. Maybe Miguel can say more on this matter, since he was at the maintainers summit, but our takeaways essentially are that we want maintainers to experiment with Rust. And if you don't have any real users, then breaking the Rust code is fine. Though I think that with breaking we mean that changes to the C side prevent the Rust side from working, not shipping Rust code without abstractions. We might be able to make an exception to the "your driver can only use abstractions" rule, but only with the promise that the subsystem is working towards creating suitable abstractions and replacing the direct C accesses with that. I personally think that we should not make that the norm, instead try to create the minimal abstraction and minimal driver (without directly calling C) that you need to start. Of course this might not work, the "minimal driver" might need to be rather complex for you to start, but I don't know your subsystem to make that judgement. >>> 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. I don't understand what you want to say. >>> (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. I also don't understand your point here. What is broken, this EROFS implementation? Why will it be painful to refactor? > ======= > 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. The longterm goal is to make everything that is possible in C, possible in Rust. For more info, please take a look at the kernel summit talk by Miguel Ojeda. However, we can only reach that longterm goal if maintainers are willing and ready to put Rust into their subsystems (either by knowing/learning Rust themselves or by having a co-maintainer that does just the Rust part). So you wanting to experiment is great. I appreciate that you also have a student working on this. Still, I think we should follow our guidelines and create abstractions in order to require as little `unsafe` code as possible. --- Cheers, Benno
diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs index 0f1400175fc2..2bd1381da5ab 100644 --- a/fs/erofs/rust/erofs_sys.rs +++ b/fs/erofs/rust/erofs_sys.rs @@ -19,4 +19,10 @@ pub(crate) type Nid = u64; /// Erofs Super Offset to read the ondisk superblock pub(crate) const EROFS_SUPER_OFFSET: Off = 1024; +/// PosixResult as a type alias to kernel::error::Result +/// to avoid naming conflicts. +pub(crate) type PosixResult<T> = Result<T, Errno>; + +pub(crate) mod errnos; pub(crate) mod superblock; +pub(crate) use errnos::Errno; diff --git a/fs/erofs/rust/erofs_sys/errnos.rs b/fs/erofs/rust/erofs_sys/errnos.rs new file mode 100644 index 000000000000..40e5cdbcb353 --- /dev/null +++ b/fs/erofs/rust/erofs_sys/errnos.rs @@ -0,0 +1,191 @@ +// Copyright 2024 Yiyang Wu +// SPDX-License-Identifier: MIT or GPL-2.0-or-later + +#[repr(i32)] +#[non_exhaustive] +#[allow(clippy::upper_case_acronyms)] +#[derive(Debug, Copy, Clone, PartialEq)] +pub(crate) enum Errno { + NONE = 0, + EPERM, + ENOENT, + ESRCH, + EINTR, + EIO, + ENXIO, + E2BIG, + ENOEXEC, + EBADF, + ECHILD, + EAGAIN, + ENOMEM, + EACCES, + EFAULT, + ENOTBLK, + EBUSY, + EEXIST, + EXDEV, + ENODEV, + ENOTDIR, + EISDIR, + EINVAL, + ENFILE, + EMFILE, + ENOTTY, + ETXTBSY, + EFBIG, + ENOSPC, + ESPIPE, + EROFS, + EMLINK, + EPIPE, + EDOM, + ERANGE, + EDEADLK, + ENAMETOOLONG, + ENOLCK, + ENOSYS, + ENOTEMPTY, + ELOOP, + ENOMSG = 42, + EIDRM, + ECHRNG, + EL2NSYNC, + EL3HLT, + EL3RST, + ELNRNG, + EUNATCH, + ENOCSI, + EL2HLT, + EBADE, + EBADR, + EXFULL, + ENOANO, + EBADRQC, + EBADSLT, + EBFONT = 59, + ENOSTR, + ENODATA, + ETIME, + ENOSR, + ENONET, + ENOPKG, + EREMOTE, + ENOLINK, + EADV, + ESRMNT, + ECOMM, + EPROTO, + EMULTIHOP, + EDOTDOT, + EBADMSG, + EOVERFLOW, + ENOTUNIQ, + EBADFD, + EREMCHG, + ELIBACC, + ELIBBAD, + ELIBSCN, + ELIBMAX, + ELIBEXEC, + EILSEQ, + ERESTART, + ESTRPIPE, + EUSERS, + ENOTSOCK, + EDESTADDRREQ, + EMSGSIZE, + EPROTOTYPE, + ENOPROTOOPT, + EPROTONOSUPPORT, + ESOCKTNOSUPPORT, + EOPNOTSUPP, + EPFNOSUPPORT, + EAFNOSUPPORT, + EADDRINUSE, + EADDRNOTAVAIL, + ENETDOWN, + ENETUNREACH, + ENETRESET, + ECONNABORTED, + ECONNRESET, + ENOBUFS, + EISCONN, + ENOTCONN, + ESHUTDOWN, + ETOOMANYREFS, + ETIMEDOUT, + ECONNREFUSED, + EHOSTDOWN, + EHOSTUNREACH, + EALREADY, + EINPROGRESS, + ESTALE, + EUCLEAN, + ENOTNAM, + ENAVAIL, + EISNAM, + EREMOTEIO, + EDQUOT, + ENOMEDIUM, + EMEDIUMTYPE, + ECANCELED, + ENOKEY, + EKEYEXPIRED, + EKEYREVOKED, + EKEYREJECTED, + EOWNERDEAD, + ENOTRECOVERABLE, + ERFKILL, + EHWPOISON, + EUNKNOWN, +} + +impl From<i32> for Errno { + fn from(value: i32) -> Self { + if (-value) <= 0 || (-value) > Errno::EUNKNOWN as i32 { + Errno::EUNKNOWN + } else { + // Safety: The value is guaranteed to be a valid errno and the memory + // layout is the same for both types. + unsafe { core::mem::transmute(value) } + } + } +} + +impl From<Errno> for i32 { + fn from(value: Errno) -> Self { + -(value as i32) + } +} + +/// Replacement for ERR_PTR in Linux Kernel. +impl From<Errno> for *const core::ffi::c_void { + fn from(value: Errno) -> Self { + (-(value as core::ffi::c_long)) as *const core::ffi::c_void + } +} + +impl From<Errno> for *mut core::ffi::c_void { + fn from(value: Errno) -> Self { + (-(value as core::ffi::c_long)) as *mut core::ffi::c_void + } +} + +/// Replacement for PTR_ERR in Linux Kernel. +impl From<*const core::ffi::c_void> for Errno { + fn from(value: *const core::ffi::c_void) -> Self { + (-(value as i32)).into() + } +} + +impl From<*mut core::ffi::c_void> for Errno { + fn from(value: *mut core::ffi::c_void) -> Self { + (-(value as i32)).into() + } +} +/// Replacement for IS_ERR in Linux Kernel. +#[inline(always)] +pub(crate) fn is_value_err(value: *const core::ffi::c_void) -> bool { + (value as core::ffi::c_ulong) >= (-4095 as core::ffi::c_long) as core::ffi::c_ulong +}
Introduce Errno to Rust side code. Note that in current Rust For Linux, Errnos are implemented as core::ffi::c_uint unit structs. However, EUCLEAN, a.k.a EFSCORRUPTED is missing from error crate. Since the errno_base hasn't changed for over 13 years, This patch merely serves as a temporary workaround for the missing errno in the Rust For Linux. Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc> --- fs/erofs/rust/erofs_sys.rs | 6 + fs/erofs/rust/erofs_sys/errnos.rs | 191 ++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 fs/erofs/rust/erofs_sys/errnos.rs