Support BufferSegments in linux kernel

[ariel-miculas]

No description provided.

[dwrensha]

So when compiling for the Linux kernel, format_args!("{x}") is a compile-time error when x is a floating point number? Is that true for format_args!("{x:?}") as well?

[ariel-miculas]

For your first question, yes:

error[E0277]: `f32` doesn't implement `Display`
  --> rust/capnp/stringify.rs:74:81
   |
74 |         dynamic_value::Reader::Float32(x) => formatter.write_fmt(format_args!("{x}")),
   |                                                                                 ^ `f32` cannot be formatted with the default formatter
   |
   = help: the trait `Display` is not implemented for `f32`
   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
   = help: the following other types implement trait `Display`:
             i128
             i16
             i32
             i64
             i8
             isize
             u128
             u16
           and 4 others
   = note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

However, I was surprised to see it is not a compile time error for format_args!("{x:?}"). I will need to test what happens in that case.

[ariel-miculas]

Trying to display a float using the debug formatting leads to a kernel panic:

        let x:f32 = 4.2;
        pr_info!("trying to display float");
        pr_info!("{}", format_args!("{x:?}"));

~ # mount -t puzzlefs none /mnt
[   10.747124] puzzlefs: trying to display float
[   10.747132] rust_kernel: panicked at 'floating point support is turned off', /home/amiculas/.rustup/toolchains/1.68.2-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/1
[   10.750707] ------------[ cut here ]------------
[   10.751557] kernel BUG at rust/helpers.c:34!
[   10.752269] invalid opcode: 0000 [#1] SMP
[   10.752892] CPU: 0 PID: 49 Comm: mount Not tainted 6.4.0-rc4+ #330
[   10.753791] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.2-1-1 04/01/2014
[   10.755058] RIP: 0010:rust_helper_BUG+0x8/0x10
[   10.755683] Code: 00 00 48 8d 7d d0 48 c7 c6 a0 da 82 81 e8 a0 20 2d 00 0f 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc f3 0f 1e fa 55 48 89 e5 <0f> 0b 66 0f 1f 44 00 00 f3 0f 1e fa 55 d
[   10.758422] RSP: 0018:ffffc90000107620 EFLAGS: 00010092
[   10.759250] RAX: 00000000000000bc RBX: ffffc90000107d30 RCX: ffffffff81a35ff0
[   10.760201] RDX: 0000000000000002 RSI: c0000000ffffefff RDI: 0000000000002ffd
[   10.760929] RBP: ffffc90000107620 R08: 0000000000000000 R09: ffffffff81a4e280
[   10.761654] R10: 00000000ffffefff R11: 0000000000000000 R12: 0000000000000000
[   10.762390] R13: 0000000000000010 R14: ffffffff8185ed30 R15: 0000000000000000
[   10.763128] FS:  0000000000f353c0(0000) GS:ffff88813bc00000(0000) knlGS:0000000000000000
[   10.763979] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   10.764589] CR2: 0000000000641fc0 CR3: 0000000100e8c002 CR4: 0000000000770eb0
[   10.765158] PKRU: 55555554
[   10.765341] Call Trace:
[   10.765504]  <TASK>
[   10.765645]  ? __die_body+0x62/0xb0
[   10.765874]  ? die+0x8c/0xb0
[   10.766065]  ? do_trap+0x87/0x150
[   10.766282]  ? rust_helper_BUG+0x8/0x10
[   10.766530]  ? handle_invalid_op+0x68/0x80
[   10.766794]  ? rust_helper_BUG+0x8/0x10
[   10.767041]  ? exc_invalid_op+0x36/0x50
[   10.767288]  ? asm_exc_invalid_op+0x1f/0x30
[   10.767559]  ? rust_helper_BUG+0x8/0x10
[   10.767806]  rust_begin_unwind+0x62/0x80
[   10.768059]  ? mas_replace+0x2b4/0x350
[   10.768302]  ? _RNvXsX_NtCsjQkEVdlX3YU_4core3fmtRNtNtCscRhiiH9rU7O_6kernel3str4CStrNtB5_7Display3fmtBz_+0xc0/0xc0
[   10.768981]  _RNvNtCsjQkEVdlX3YU_4core9panicking9panic_fmt+0x2d/0x30
[   10.769490]  _RNvXNtNtCsjQkEVdlX3YU_4core3fmt7nofloatfNtB4_5Debug3fmt+0x40/0x50
[   10.769981]  ? call_rcu+0xd4/0x220
[   10.770165]  _RNvNtCsjQkEVdlX3YU_4core3fmt5write+0x1ba/0x220
[   10.770389]  _RNvXs5_NtCsjQkEVdlX3YU_4core3fmtNtB5_9ArgumentsNtB5_7Display3fmt+0x4a/0x50
[   10.770703]  ? _RINvNtCsjQkEVdlX3YU_4core3ptr13drop_in_placeINtNtNtCscRhiiH9rU7O_6kernel2fs5param15ConcreteHandleruNCINvNtBJ_3s327handleruE0EECshR6aa2uJhw8_8puzzlefs+0x10/0x10
[   10.771303]  _RNvNtCsjQkEVdlX3YU_4core3fmt5write+0x1ba/0x220
[   10.771527]  rust_fmt_argument+0x5f/0x70
[   10.771683]  ? _RINvNtCsjQkEVdlX3YU_4core3ptr13drop_in_placeINtNtNtCscRhiiH9rU7O_6kernel2fs5param15ConcreteHandleruNCINvNtBJ_3s327handleruE0EECshR6aa2uJhw8_8puzzlefs+0x10/0x10
[   10.772286]  pointer+0x4c6/0x6a0
[   10.772418]  vsnprintf+0x493/0x730
[   10.772557]  vprintk_store+0x194/0x5a0
[   10.772708]  ? console_unlock+0xe1/0x100
[   10.772865]  vprintk_emit+0x65/0x1c0
[   10.773009]  vprintk_default+0x1c/0x20
[   10.773159]  vprintk+0x4d/0x60
[   10.773284]  _printk+0x4a/0x50
[   10.773408]  _RNvXCshR6aa2uJhw8_8puzzlefsNtB2_8PuzzleFsNtNtCscRhiiH9rU7O_6kernel2fs4Type10fill_super+0x50b/0x520
[   10.773801]  ? _RINvNtCsjQkEVdlX3YU_4core3ptr13drop_in_placeINtNtNtCscRhiiH9rU7O_6kernel2fs5param15ConcreteHandleruNCINvNtBJ_3s327handleruE0EECshR6aa2uJhw8_8puzzlefs+0x10/0x10
[   10.774399]  ? _RINvNtCsjQkEVdlX3YU_4core3ptr13drop_in_placeINtNtNtCscRhiiH9rU7O_6kernel2fs5param15ConcreteHandleruNCINvNtBJ_3s327handleruE0EECshR6aa2uJhw8_8puzzlefs+0x10/0x10
[   10.774997]  ? _RINvNtCsjQkEVdlX3YU_4core3ptr13drop_in_placeINtNtNtCscRhiiH9rU7O_6kernel2fs5param15ConcreteHandleruNCINvNtBJ_3s327handleruE0EECshR6aa2uJhw8_8puzzlefs+0x10/0x10
[   10.775598]  ? _RINvNtCsjQkEVdlX3YU_4core3ptr13drop_in_placeINtNtNtCscRhiiH9rU7O_6kernel2fs5param15ConcreteHandleruNCINvNtBJ_3s327handleruE0EECshR6aa2uJhw8_8puzzlefs+0x10/0x10
[   10.776195]  ? _RNvXs4_NtCsjQkEVdlX3YU_4core3fmtNtB5_9ArgumentsNtB5_5Debug3fmt+0x50/0x50
[   10.776508]  ? _RNvXs13_NtNtCsjQkEVdlX3YU_4core4sync6atomicNtB6_11AtomicUsizeNtNtBa_3fmt5Debug3fmt+0x20/0x20
[   10.776888]  ? _RNvMNtCscRhiiH9rU7O_6kernel2fsINtB2_6TablesNtCshR6aa2uJhw8_8puzzlefs8PuzzleFsE17get_tree_callbackBH_+0x20/0x20
[   10.777325]  _RNvMNtCscRhiiH9rU7O_6kernel2fsINtB2_6TablesNtCshR6aa2uJhw8_8puzzlefs8PuzzleFsE19fill_super_callbackBH_+0x29/0x50
[   10.777763]  ? _RNvMNtCscRhiiH9rU7O_6kernel2fsINtB2_6TablesNtCshR6aa2uJhw8_8puzzlefs8PuzzleFsE17get_tree_callbackBH_+0x20/0x20
[   10.778199]  vfs_get_super+0x8c/0x120
[   10.778346]  get_tree_nodev+0x14/0x20
[   10.778492]  vfs_get_tree+0x20/0x90
[   10.778635]  do_new_mount+0x140/0x340
[   10.778784]  path_mount+0x384/0x620
[   10.778924]  __x64_sys_mount+0x142/0x1a0
[   10.779081]  do_syscall_64+0x48/0x90
[   10.779224]  entry_SYSCALL_64_after_hwframe+0x46/0xb0
[   10.779423] RIP: 0033:0x4958ce
[   10.779547] Code: 48 c7 c1 e0 ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e0 8
[   10.780314] RSP: 002b:00007ffcabbf9228 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[   10.780608] RAX: ffffffffffffffda RBX: 00007ffcabbf93f8 RCX: 00000000004958ce
[   10.780886] RDX: 00007ffcabbf9fba RSI: 00007ffcabbf9fc8 RDI: 00007ffcabbf9fc3
[   10.781163] RBP: 00007ffcabbf9fc3 R08: 0000000000000000 R09: 0000000000000000
[   10.781440] R10: 0000000000008000 R11: 0000000000000246 R12: 00007ffcabbf9fc8
[   10.781717] R13: 00007ffcabbf9fba R14: 0000000000008000 R15: 00007ffcabbf9720
[   10.782007]  </TASK>
[   10.782097] Modules linked in:
[   10.782222] ---[ end trace 0000000000000000 ]---
[   10.782404] RIP: 0010:rust_helper_BUG+0x8/0x10
[   10.782581] Code: 00 00 48 8d 7d d0 48 c7 c6 a0 da 82 81 e8 a0 20 2d 00 0f 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc f3 0f 1e fa 55 48 89 e5 <0f> 0b 66 0f 1f 44 00 00 f3 0f 1e fa 55 d
[   10.783295] RSP: 0018:ffffc90000107620 EFLAGS: 00010092
[   10.783499] RAX: 00000000000000bc RBX: ffffc90000107d30 RCX: ffffffff81a35ff0
[   10.783775] RDX: 0000000000000002 RSI: c0000000ffffefff RDI: 0000000000002ffd
[   10.784053] RBP: ffffc90000107620 R08: 0000000000000000 R09: ffffffff81a4e280
[   10.784331] R10: 00000000ffffefff R11: 0000000000000000 R12: 0000000000000000
[   10.784609] R13: 0000000000000010 R14: ffffffff8185ed30 R15: 0000000000000000
[   10.784886] FS:  0000000000f353c0(0000) GS:ffff88813bc00000(0000) knlGS:0000000000000000
[   10.785198] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   10.785425] CR2: 0000000000641fc0 CR3: 0000000100e8c002 CR4: 0000000000770eb0
[   10.785703] PKRU: 55555554
[   10.785812] note: mount[49] exited with irqs disabled
Segmentation fault

[dwrensha]

I'd like to avoid adding yet another feature flag, if possible.

The added error handling on SegmentLengthsBuilder seems fine to add without hiding it behind a feature flag.

I suppose the floating-point formatting issue might be more tricky.

[ariel-miculas]

The added error handling on SegmentLengthsBuilder seems fine to add without hiding it behind a feature flag.

Unfortunately there's no try_with_capacity in alloc, it exists only in the kernel. Maybe we could use something like https://crates.io/crates/fallible_vec (mentioned here: rust-lang/rust#95051)
@ojeda do you have any ideas for a more elegant approach?

[dwrensha]

I was hoping that read_message_from_flat_slice_no_alloc() might be sufficient for your kernel-related use cases. Can you explain why SegmentLengthsBuilder is needed?

[ariel-miculas]

It's related to #259, this is what I have in userspace: https://github.com/ariel-miculas/puzzlefs/blob/capnproto/format/src/types.rs#L707-L726
I'm storing the reader in MetadataBlob, and then using the reader in https://github.com/ariel-miculas/puzzlefs/blob/capnproto/format/src/types.rs#L728 to read the inodes.
I wanted to avoid having to call read_message_from_flat_slice_no_alloc() every time, so that's why I stored the reader.
In the kernel I would like to have the same code as in userspace, as much as possible.

[ojeda]

Unfortunately there's no try_with_capacity in alloc, it exists only in the kernel. Maybe we could use something like https://crates.io/crates/fallible_vec (mentioned here: rust-lang/rust#95051) @ojeda do you have any ideas for a more elegant approach?

It wouldn't hurt trying to upstream try_with_capacity, it may be easier since it wouldn't extend the API that much. Do you need other methods?

[ariel-miculas]

It wouldn't hurt trying to upstream try_with_capacity, it may be easier since it wouldn't extend the API that much. Do you need other methods?

I also need try_resize and try_push. I see there was an attempt to upstream them here: rust-lang/rust#95051

[ojeda]

Yeah, that PR added a lot of APIs, but perhaps upstreaming a few would be OK for the time being (this is what I meant by "it may be easier", i.e. I was comparing it to that PR, not to fallible_vec), and may help restarting the discussion around fallible allocations etc.

Cc @Ericson2314 @dpaoliello since they may have some news on their side.

[dwrensha]

@ariel-miculas another idea could be to add a new structure BufferSegmentsNoAlloc which is like BufferSegments, but instead of having a Vec of segment indices would just store the offset to the start of the segments. The new structure would avoid allocation, at the cost of needing to add up all segments lengths up to n on each call to get_segment(n). (As far as I am aware, messages with more than a handful of segments are rare.)

[dpaoliello]

It wouldn't hurt trying to upstream try_with_capacity, it may be easier since it wouldn't extend the API that much. Do you need other methods?

I also need try_resize and try_push. I see there was an attempt to upstream them here: rust-lang/rust#95051

All of those APIs are available in fallible_vec.

Yeah, that PR added a lot of APIs, but perhaps upstreaming a few would be OK for the time being (this is what I meant by "it may be easier", i.e. I was comparing it to that PR, not to fallible_vec), and may help restarting the discussion around fallible allocations etc.

Cc @Ericson2314 @dpaoliello since they may have some news on their side.

That PR is dead: the library team pushed back (quite fairly) on adding so many APIs. The fallible_vec crate is the short term workaround, and I'm hoping that keyword generics will allow us an elegant way to add them to the standard library in the future.

[ojeda]

Related: rust-lang/rust#111970.

[ariel-miculas]

@ariel-miculas another idea could be to add a new structure BufferSegmentsNoAlloc which is like BufferSegments, but instead of having a Vec of segment indices would just store the offset to the start of the segments. The new structure would avoid allocation, at the cost of needing to add up all segments lengths up to n on each call to get_segment(n). (As far as I am aware, messages with more than a handful of segments are rare.)

I think this would be the most feasible solution, I'll give it a try. I don't want to add fallible_vec as a dependency, it doesn't seem right for no_std environments.

[dpaoliello]

I think this would be the most feasible solution, I'll give it a try. I don't want to add fallible_vec as a dependency, it doesn't seem right for no_std environments.

@ariel-miculas Not trying to argue one way or the other, but I'm curious why you believe that fallible_vec isn't right for no_std environments (assuming that you are already depending on Vec and the alloc crate)? It was explicitly designed to work in no_std + alloc with #[cfg(no_global_oom_handling)] enabled.

[ariel-miculas]

It doesn't seem right for my use case, which is to add capnp in the linux kernel. In there, I already have the fallible allocation APIs. So it feels to me I would be adding a "phantom" dependency if I were to include fallible_vec. The only reason would be so the APIs in the kernel compile in non-kernel environments. And in the end, I wouldn't accomplish anything in practice: I could only enable alloc in the kernel if I would change every infallible allocation to a fallible allocation. Even then, it wouldn't work because the kernel doesn't have all the APIs in the fallible_vec crate. In conclusion, I would still need the "kernel" feature flag in addition to "alloc" flag in order to fine tune which parts of the capnp API I could use.
It's just easier to create another no_alloc API and use that in the kernel environment.

[dwrensha]

@ariel-miculas: since #423 has landed, is this PR still needed?

[ariel-miculas]

No, I'll close it, thanks.