Messages in this thread | | | Date | Wed, 29 Nov 2023 16:42:51 +0000 | Subject | Re: [PATCH 1/7] rust: file: add Rust abstraction for `struct file` | From | Alice Ryhl <> |
| |
Matthew Wilcox <willy@infradead.org> > I haven't looked at how Rust-for-Linux handles errors yet, but it's > disappointing to see that it doesn't do something like the PTR_ERR / > ERR_PTR / IS_ERR C thing under the hood.
It would be cool to do that, but we haven't written the infrastructure to do that yet. (Note that in this particular case, the C function also returns the error as a null pointer.)
>> @@ -157,6 +158,12 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func, >> } >> EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key); >> >> +struct file *rust_helper_get_file(struct file *f) >> +{ >> + return get_file(f); >> +} >> +EXPORT_SYMBOL_GPL(rust_helper_get_file); > > This is ridiculous. A function call instead of doing the > atomic_long_inc() in Rust?
I think there are two factors to consider here:
First, doing the atomic increment from Rust currently runs into the memory model split between the C++ and LKMM memory models. It would be like using the C11 atomic_fetch_add instead of the one that the Kernel defines for LKMM using inline assembly. When I discussed this with Paul McKenney, we were advised that its best to avoid mixing the memory models.
Avoiding this would require that we replicate the inline assembly that C uses to define its atomic operations on the Rust side. This is something that I think should be done, but it hasn't been done yet.
Second, there's potentially an increased maintenance burden when C methods are reimplemented in Rust. Any change to the implementation on the C side would have to be reflected on the Rust side.
Alice
| |