Support BufferSegments in linux kernel by ariel-miculas · Pull Request #422 · capnproto/capnproto-rust

ariel-miculas · 2023-07-04T13:37:50Z

No description provided.

The linux kernel doesn't support infallible allocations, so it has its own special methods for allocating memory (e.g. try_with_capacity, try_push). This implies that the functions that allocate memory must return a result.

…inux kernel

dwrensha · 2023-07-04T14:06:13Z

capnp/src/stringify.rs

+        #[cfg(not(feature = "kernel"))]
        dynamic_value::Reader::Float64(x) => formatter.write_fmt(format_args!("{x}")),
+        #[cfg(feature = "kernel")]
+        dynamic_value::Reader::Float32(_) | dynamic_value::Reader::Float64(_) => Err(fmt::Error),


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?

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.

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 · 2023-07-04T14:10:41Z

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 · 2023-07-04T15:21:23Z

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 · 2023-07-04T15:49:21Z

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 · 2023-07-04T16:08:24Z

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 · 2023-07-04T16:26:14Z

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 · 2023-07-04T16:48:47Z

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 · 2023-07-04T17:14:31Z

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 · 2023-07-04T17:25:35Z

@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 · 2023-07-04T19:39:16Z

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 · 2023-07-04T19:43:49Z

Related: rust-lang/rust#111970.

ariel-miculas · 2023-07-05T08:50:48Z

@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 · 2023-07-05T15:20:30Z

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 · 2023-07-05T15:46:39Z

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 · 2023-08-30T01:34:09Z

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

ariel-miculas · 2023-08-30T05:34:48Z

No, I'll close it, thanks.

ariel-miculas added 2 commits July 4, 2023 16:26

Adapt BufferSegments for usage in the linux kernel

5364e12

The linux kernel doesn't support infallible allocations, so it has its own special methods for allocating memory (e.g. try_with_capacity, try_push). This implies that the functions that allocate memory must return a result.

Return an error when trying to display floating point values in the l…

b5b94b1

…inux kernel

dwrensha reviewed Jul 4, 2023

View reviewed changes

ariel-miculas closed this Aug 30, 2023

Conversation

ariel-miculas commented Jul 4, 2023

Uh oh!

dwrensha Jul 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ariel-miculas Jul 4, 2023

Choose a reason for hiding this comment

Uh oh!

ariel-miculas Jul 4, 2023

Choose a reason for hiding this comment

Uh oh!

dwrensha commented Jul 4, 2023

Uh oh!

ariel-miculas commented Jul 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwrensha commented Jul 4, 2023

Uh oh!

ariel-miculas commented Jul 4, 2023

Uh oh!

ojeda commented Jul 4, 2023

Uh oh!

ariel-miculas commented Jul 4, 2023

Uh oh!

ojeda commented Jul 4, 2023

Uh oh!

dwrensha commented Jul 4, 2023

Uh oh!

dpaoliello commented Jul 4, 2023

Uh oh!

ojeda commented Jul 4, 2023

Uh oh!

ariel-miculas commented Jul 5, 2023

Uh oh!

dpaoliello commented Jul 5, 2023

Uh oh!

ariel-miculas commented Jul 5, 2023

Uh oh!

dwrensha commented Aug 30, 2023

Uh oh!

ariel-miculas commented Aug 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dwrensha Jul 4, 2023 •

edited

Loading

ariel-miculas commented Jul 4, 2023 •

edited

Loading