[v2] rust: error: add missing error codes

  This adds the error codes from `include/linux/errno.h` to the list of
Rust error constants. These errors were not included originally, because
they are not supposed to be visible from userspace. However, they are
still a perfectly valid error to use when writing a kernel driver. For
example, you might want to return ERESTARTSYS if you receive a signal
during a call to `schedule`.

This patch inserts an annotation to skip rustfmt on the list of error
codes. Without it, three of the error codes are split over several
lines, which looks terribly inconsistent.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/error.rs | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)


base-commit: ea76e08f4d901a450619831a255e9e0a4c0ed162
  

On 5/4/23 03:48, Alice Ryhl wrote:
> This adds the error codes from `include/linux/errno.h` to the list of
> Rust error constants. These errors were not included originally, because
> they are not supposed to be visible from userspace. However, they are
> still a perfectly valid error to use when writing a kernel driver. For
> example, you might want to return ERESTARTSYS if you receive a signal
> during a call to `schedule`.
> 
> This patch inserts an annotation to skip rustfmt on the list of error
> codes. Without it, three of the error codes are split over several
> lines, which looks terribly inconsistent.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>   rust/kernel/error.rs | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
  

On Thu,  4 May 2023 06:48:54 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> This adds the error codes from `include/linux/errno.h` to the list of
> Rust error constants. These errors were not included originally, because
> they are not supposed to be visible from userspace. However, they are
> still a perfectly valid error to use when writing a kernel driver. For
> example, you might want to return ERESTARTSYS if you receive a signal
> during a call to `schedule`.

`include/linux/errno.h` also includes all of `asm/errno.h`,
which defines EDEADLK - EHWPOISON, which is not included in this patch.
I feel like these error codes should be added first?

> 
> This patch inserts an annotation to skip rustfmt on the list of error
> codes. Without it, three of the error codes are split over several
> lines, which looks terribly inconsistent.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/error.rs | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 5f4114b30b94..de4fa8640f29 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -14,6 +14,7 @@ use core::num::TryFromIntError;
>  use core::str::Utf8Error;
>  
>  /// Contains the C-compatible error codes.
> +#[rustfmt::skip]
>  pub mod code {
>      macro_rules! declare_err {
>          ($err:tt $(,)? $($doc:expr),+) => {
> @@ -58,6 +59,25 @@ pub mod code {
>      declare_err!(EPIPE, "Broken pipe.");
>      declare_err!(EDOM, "Math argument out of domain of func.");
>      declare_err!(ERANGE, "Math result not representable.");
> +    declare_err!(ERESTARTSYS, "Restart the system call.");
> +    declare_err!(ERESTARTNOINTR, "System call was interrupted by a signal and will be restarted.");
> +    declare_err!(ERESTARTNOHAND, "Restart if no handler.");
> +    declare_err!(ENOIOCTLCMD, "No ioctl command.");
> +    declare_err!(ERESTART_RESTARTBLOCK, "Restart by calling sys_restart_syscall.");
> +    declare_err!(EPROBE_DEFER, "Driver requests probe retry.");
> +    declare_err!(EOPENSTALE, "Open found a stale dentry.");
> +    declare_err!(ENOPARAM, "Parameter not supported.");
> +    declare_err!(EBADHANDLE, "Illegal NFS file handle.");
> +    declare_err!(ENOTSYNC, "Update synchronization mismatch.");
> +    declare_err!(EBADCOOKIE, "Cookie is stale.");
> +    declare_err!(ENOTSUPP, "Operation is not supported.");
> +    declare_err!(ETOOSMALL, "Buffer or request is too small.");
> +    declare_err!(ESERVERFAULT, "An untranslatable error occurred.");
> +    declare_err!(EBADTYPE, "Type not supported by server.");
> +    declare_err!(EJUKEBOX, "Request initiated, but will not complete before timeout.");
> +    declare_err!(EIOCBQUEUED, "iocb queued, will get completion event.");
> +    declare_err!(ERECALLCONFLICT, "Conflict with recalled state.");
> +    declare_err!(ENOGRACE, "NFS file lock reclaim refused.");
>  }
>  
>  /// Generic integer kernel error.
> 
> base-commit: ea76e08f4d901a450619831a255e9e0a4c0ed162
  

On Mon, 8 May 2023 12:47:01 +0100
Gary Guo <gary@garyguo.net> wrote:
> On Thu,  4 May 2023 06:48:54 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
> > This adds the error codes from `include/linux/errno.h` to the list of
> > Rust error constants. These errors were not included originally, because
> > they are not supposed to be visible from userspace. However, they are
> > still a perfectly valid error to use when writing a kernel driver. For
> > example, you might want to return ERESTARTSYS if you receive a signal
> > during a call to `schedule`.
> 
> `include/linux/errno.h` also includes all of `asm/errno.h`,
> which defines EDEADLK - EHWPOISON, which is not included in this patch.
> I feel like these error codes should be added first?

It seems like there are a lot of asm/errno.h files:

$ find . -name errno.h
./arch/powerpc/include/uapi/asm/errno.h
./arch/mips/include/asm/errno.h
./arch/mips/include/uapi/asm/errno.h
./arch/alpha/include/uapi/asm/errno.h
./arch/parisc/include/uapi/asm/errno.h
./arch/sparc/include/uapi/asm/errno.h
./arch/x86/include/generated/uapi/asm/errno.h
./tools/arch/powerpc/include/uapi/asm/errno.h
./tools/arch/mips/include/asm/errno.h
./tools/arch/mips/include/uapi/asm/errno.h
./tools/arch/alpha/include/uapi/asm/errno.h
./tools/arch/parisc/include/uapi/asm/errno.h
./tools/arch/sparc/include/uapi/asm/errno.h
./tools/arch/x86/include/uapi/asm/errno.h
./tools/include/nolibc/errno.h
./tools/include/uapi/asm/errno.h
./tools/include/uapi/asm-generic/errno.h
./include/uapi/asm-generic/errno.h
./include/uapi/linux/errno.h
./include/linux/errno.h

How should I proceed with this? You mention EDEADLK - EHWPOISON, but its
not clear to me which asm/errno.h file I should base this on.
  

On Tue, May 09, 2023 at 08:07:00AM +0000, Alice Ryhl wrote:
> On Mon, 8 May 2023 12:47:01 +0100
> Gary Guo <gary@garyguo.net> wrote:
> > On Thu,  4 May 2023 06:48:54 +0000
> > Alice Ryhl <aliceryhl@google.com> wrote:
> > > This adds the error codes from `include/linux/errno.h` to the list of
> > > Rust error constants. These errors were not included originally, because
> > > they are not supposed to be visible from userspace. However, they are
> > > still a perfectly valid error to use when writing a kernel driver. For
> > > example, you might want to return ERESTARTSYS if you receive a signal
> > > during a call to `schedule`.
> > 
> > `include/linux/errno.h` also includes all of `asm/errno.h`,
> > which defines EDEADLK - EHWPOISON, which is not included in this patch.
> > I feel like these error codes should be added first?
> 
> It seems like there are a lot of asm/errno.h files:
> 
> $ find . -name errno.h
> ./arch/powerpc/include/uapi/asm/errno.h
> ./arch/mips/include/asm/errno.h
> ./arch/mips/include/uapi/asm/errno.h
> ./arch/alpha/include/uapi/asm/errno.h
> ./arch/parisc/include/uapi/asm/errno.h
> ./arch/sparc/include/uapi/asm/errno.h
> ./arch/x86/include/generated/uapi/asm/errno.h
> ./tools/arch/powerpc/include/uapi/asm/errno.h
> ./tools/arch/mips/include/asm/errno.h
> ./tools/arch/mips/include/uapi/asm/errno.h
> ./tools/arch/alpha/include/uapi/asm/errno.h
> ./tools/arch/parisc/include/uapi/asm/errno.h
> ./tools/arch/sparc/include/uapi/asm/errno.h
> ./tools/arch/x86/include/uapi/asm/errno.h
> ./tools/include/nolibc/errno.h
> ./tools/include/uapi/asm/errno.h
> ./tools/include/uapi/asm-generic/errno.h

You can ignore the tool ones.

> ./include/uapi/asm-generic/errno.h
> ./include/uapi/linux/errno.h
> ./include/linux/errno.h
> 
> How should I proceed with this? You mention EDEADLK - EHWPOISON, but its
> not clear to me which asm/errno.h file I should base this on.

It depends on which arch you are building for.  That's why we have
per-platform errno.h files, the values are different for different ones.
So you need to handle them all properly somehow.  How is rust going to
handle per-arch stuff like this?

thanks,

greg k-h
  

On Tue, May 9, 2023 at 10:46 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> It depends on which arch you are building for.  That's why we have
> per-platform errno.h files, the values are different for different ones.
> So you need to handle them all properly somehow.  How is rust going to
> handle per-arch stuff like this?

We can do conditional compilation in the same file, possibly with a
Rust macro which takes a nice table that shows all arches at once.

We can also split into files like C and move it to each `arch/`, there
are a couple of approaches for this. This is best for `MAINTAINERS`,
although these headers almost never change, so it is not a big
advantage.

We could also automatically do everything based on the C headers, too.
Back then it felt to me like too much complexity for little gain,
given those C headers almost never change, but now it may be worth it.
Or, instead, having a test that verifies they are the same instead,
and that way we don't introduce complexity for the build itself.

Alice only needs `ERESTARTSYS` so far, as far as I understand, so
perhaps it is simplest to only add the rest of the non-generic ones
for the moment; and gather opinions on the approaches above meanwhile.

Cheers,
Miguel
  

Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:

> On Tue, May 9, 2023 at 10:46 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> It depends on which arch you are building for.  That's why we have
>> per-platform errno.h files, the values are different for different ones.
>> So you need to handle them all properly somehow.  How is rust going to
>> handle per-arch stuff like this?
>
> We can do conditional compilation in the same file, possibly with a
> Rust macro which takes a nice table that shows all arches at once.
>
> We can also split into files like C and move it to each `arch/`, there
> are a couple of approaches for this. This is best for `MAINTAINERS`,
> although these headers almost never change, so it is not a big
> advantage.
>
> We could also automatically do everything based on the C headers, too.
> Back then it felt to me like too much complexity for little gain,
> given those C headers almost never change, but now it may be worth it.
> Or, instead, having a test that verifies they are the same instead,
> and that way we don't introduce complexity for the build itself.
>
> Alice only needs `ERESTARTSYS` so far, as far as I understand, so
> perhaps it is simplest to only add the rest of the non-generic ones
> for the moment; and gather opinions on the approaches above meanwhile.

Let's add the ones we need for now. When we need target specific error
codes we can have a `mod` for each arch, gate behind the target
feature and conditionally reexport them.

BR Andreas
  

On Thu, May 4, 2023 at 8:49 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> This adds the error codes from `include/linux/errno.h` to the list of
> Rust error constants. These errors were not included originally, because
> they are not supposed to be visible from userspace. However, they are
> still a perfectly valid error to use when writing a kernel driver. For
> example, you might want to return ERESTARTSYS if you receive a signal
> during a call to `schedule`.
>
> This patch inserts an annotation to skip rustfmt on the list of error
> codes. Without it, three of the error codes are split over several
> lines, which looks terribly inconsistent.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Applied to `rust-next` -- thanks everyone!

Cheers,
Miguel