Current for consts that bindgen don't recognise, we define a helper
constant with
const <TYPE> BINDINGS_<NAME> = <NAME>;
in `bindings_helper.h` and then we put
pub const <NAME>: <TYPE> = BINDINGS_<NAME>;
in `bindings/lib.rs`. This is fine that we currently only have 3
constants that are defined this way, but is going to be more annoying
when more constants are added since every new constant needs to be
defined in two places.
This patch changes the way we define constant helpers to
const <TYPE> RUST_CONST_HELPER_<NAME> = <NAME>;
and then use `sed` to postprocess Rust code by generated by bindgen to
remove the distinct prefix, so user of the binding crate can refer to
the name directly.
Reviewed-by: Benno Lossin <[email protected]>
Reviewed-by: Andreas Hindborg <[email protected]>
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
Signed-off-by: Gary Guo <[email protected]>
---
v1 -> v2:
- Changed `RUST_BINDING_` to `RUST_CONST_HELPER_`.
rust/Makefile | 2 ++
rust/bindings/bindings_helper.h | 6 +++---
rust/bindings/lib.rs | 3 ---
rust/kernel/allocator.rs | 2 +-
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index a27f35f924ec..57f7f5e5a95d 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -337,6 +337,8 @@ quiet_cmd_bindgen = BINDGEN $@
$(obj)/bindings/bindings_generated.rs: private bindgen_target_flags = \
$(shell grep -Ev '^#|^$$' $(srctree)/$(src)/bindgen_parameters)
+$(obj)/bindings/bindings_generated.rs: private bindgen_target_extra = ; \
+ sed -Ei 's/pub const RUST_CONST_HELPER_([a-zA-Z0-9_]*)/pub const \1/g' $@
$(obj)/bindings/bindings_generated.rs: $(src)/bindings/bindings_helper.h \
$(src)/bindgen_parameters FORCE
$(call if_changed_dep,bindgen)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index c91a3c24f607..bfa0fa794dae 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -14,6 +14,6 @@
#include <linux/sched.h>
/* `bindgen` gets confused at certain things. */
-const size_t BINDINGS_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
-const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
-const gfp_t BINDINGS___GFP_ZERO = __GFP_ZERO;
+const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
+const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
+const gfp_t RUST_CONST_HELPER___GFP_ZERO = __GFP_ZERO;
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 9bcbea04dac3..40ddaee50d8b 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -48,6 +48,3 @@ mod bindings_helper {
}
pub use bindings_raw::*;
-
-pub const GFP_KERNEL: gfp_t = BINDINGS_GFP_KERNEL;
-pub const __GFP_ZERO: gfp_t = BINDINGS___GFP_ZERO;
diff --git a/rust/kernel/allocator.rs b/rust/kernel/allocator.rs
index a8f3d5be1af1..4b057e837358 100644
--- a/rust/kernel/allocator.rs
+++ b/rust/kernel/allocator.rs
@@ -21,7 +21,7 @@ unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: bindings::gf
let mut size = layout.size();
- if layout.align() > bindings::BINDINGS_ARCH_SLAB_MINALIGN {
+ if layout.align() > bindings::ARCH_SLAB_MINALIGN {
// The alignment requirement exceeds the slab guarantee, thus try to enlarge the size
// to use the "power-of-two" size/alignment guarantee (see comments in `kmalloc()` for
// more information).
base-commit: 3857af38e57a80b15b994e19d1f4301cac796481
--
2.40.1
On 11/4/23 15:56, Gary Guo wrote:
> Current for consts that bindgen don't recognise, we define a helper
> constant with
>
> const <TYPE> BINDINGS_<NAME> = <NAME>;
>
> in `bindings_helper.h` and then we put
>
> pub const <NAME>: <TYPE> = BINDINGS_<NAME>;
>
> in `bindings/lib.rs`. This is fine that we currently only have 3
> constants that are defined this way, but is going to be more annoying
> when more constants are added since every new constant needs to be
> defined in two places.
>
> This patch changes the way we define constant helpers to
>
> const <TYPE> RUST_CONST_HELPER_<NAME> = <NAME>;
>
> and then use `sed` to postprocess Rust code by generated by bindgen to
> remove the distinct prefix, so user of the binding crate can refer to
> the name directly.
>
> Reviewed-by: Benno Lossin <[email protected]>
> Reviewed-by: Andreas Hindborg <[email protected]>
> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
> Signed-off-by: Gary Guo <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
On 04.11.23 15:56, Gary Guo wrote:
> diff --git a/rust/Makefile b/rust/Makefile
> index a27f35f924ec..57f7f5e5a95d 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -337,6 +337,8 @@ quiet_cmd_bindgen = BINDGEN $@
>
> $(obj)/bindings/bindings_generated.rs: private bindgen_target_flags = \
> $(shell grep -Ev '^#|^$$' $(srctree)/$(src)/bindgen_parameters)
> +$(obj)/bindings/bindings_generated.rs: private bindgen_target_extra = ; \
> + sed -Ei 's/pub const RUST_CONST_HELPER_([a-zA-Z0-9_]*)/pub const \1/g' $@
I think I mentioned this before, but I think this should use `^`
to ensure that this is only replaced at the beginning of lines.
--
Cheers,
Benno
On Sat, Nov 4, 2023 at 3:58 PM Gary Guo <[email protected]> wrote:
>
> This patch changes the way we define constant helpers to
>
> const <TYPE> RUST_CONST_HELPER_<NAME> = <NAME>;
>
> and then use `sed` to postprocess Rust code by generated by bindgen to
> remove the distinct prefix, so user of the binding crate can refer to
> the name directly.
>
> Reviewed-by: Benno Lossin <[email protected]>
> Reviewed-by: Andreas Hindborg <[email protected]>
> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
> Signed-off-by: Gary Guo <[email protected]>
Applied to `rust-next` (reworded for typos and with Benno's `^`
suggestion -- we can always relax it back later if needed, the output
is currently the same).
Thanks everyone!
Cheers,
Miguel
On Wed, Dec 13, 2023 at 7:44 PM Miguel Ojeda
<[email protected]> wrote:
>
> Applied to `rust-next` (reworded for typos and with Benno's `^`
> suggestion -- we can always relax it back later if needed, the output
> is currently the same).
I have dropped the `^` suggestion from `rust-next` since that breaks
non-`rustfmt` builds [1] given everything goes in a single line in the
generated code.
We may want to consider requiring `rustfmt` at some point to simplify things.
[1] https://storage.kernelci.org/next/master/next-20231214/x86_64/x86_64_defconfig+rust/rustc-1.73/logs/kernel.log
Cheers,
Miguel