Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

From: Miguel Ojeda
Date: Wed Jan 08 2025 - 09:47:46 EST

Next message: Guenter Roeck: "Re: [PATCH v2 1/3] hwmon: (acpi_power_meter) Fix uninitialized variables"
Previous message: Loic Poulain: "Re: [PATCH 1/2] bus: mhi: host: pci_generic: Use pci_try_reset_function() to avoid deadlock"
In reply to: Viresh Kumar: "Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Jan 7, 2025 at 12:22 PM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> Rust isn't able to parse the macro for now, avoid using it.

To clarify, the thing that struggles with `BIT` is `bindgen`, rather than Rust.

The reason is that `bindgen` does not handle non-trivial `#define`s.
The good news is that recently work got merged to make `bindgen`
understand those that resolve to a constant by (essentially) asking
the C compiler to do so. It is the `--clang-macro-fallback` flag.

Since I wanted to try it within the kernel for a while, I just did so
for `BIT` here, and it works in an standalone header, e.g.

#include <vdso/bits.h>
#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY BIT(3)

gives me:

pub const CPUFREQ_HAVE_GOVERNOR_PER_POLICY: u32 = 8;

with `bindgen` 0.71.1.

However, when I tried to make it work within the full kernel build, it
did not work. I reduced it to using `-include` flags for the C
compiler (which we pass a few in the kernel build). I created an issue
at:

https://github.com/rust-lang/rust-bindgen/issues/3069

I confirmed this by including them manually at the top of the
`bindings_helper.h` file, and it worked (I get about ~1500 extra
constants). It requires something like:

#ifdef __BINDGEN__
#include <linux/compiler-version.h>
#include <linux/kconfig.h>
#include <linux/compiler_types.h>
#endif

Plus filtering out the flags.

See the attached patch in case someone wants to play with the feature
and/or improve `bindgen`. Note: it is not a finalized patch at all,
and not intended to be applied, but it gets you a build with the
`CPUFREQ` constant available.

So we could, in principle, use it that way as a workaround. Having
said that, we still need to upgrade the `bindgen` minimum supported
version. So it may be best to simply fix the issue upstream and then
make the upgrade eventually -- `--clang-macro-fallback` is a new
feature anyway.

I also noticed a couple other issues, which strengths that suggestion,
i.e. to try to fix the sharp edges or at least get an agreed
workaround before start using the feature:

- The dependencies file (`-Wp,-MMD,file.d`) changing behavior: I
filed https://github.com/rust-lang/rust-bindgen/issues/3070.

- Duplicate definitions created (`pub const`s), e.g. for
https://elixir.bootlin.com/linux/v6.12.6/source/include/uapi/linux/pkt_sched.h#L598-L604.

I filed https://github.com/rust-lang/rust-bindgen/issues/3071 (it
may be related to
https://github.com/rust-lang/rust-bindgen/issues/3066 too).

I hope that helps!

(Cc'ing John, Christian and Emilio)

Cheers,
Miguel
From 35b761799526a54cf63bf6bbb8508fd05091bcbd Mon Sep 17 00:00:00 2001
From: Miguel Ojeda <ojeda@xxxxxxxxxx>
Date: Tue, 7 Jan 2025 19:55:17 +0000
Subject: [PATCH] TEST rust: try `--clang-macro-feedback`

Only intended to test the feature, with workarounds that we probably do
not want to have.

Only briefly build-tested for `srctree` builds.

Link: https://lore.kernel.org/rust-for-linux/9719ba8b3a921bd9f2cb7ebf902c54c708b5409d.1736248242.git.viresh.kumar@xxxxxxxxxx/
Signed-off-by: Miguel Ojeda <ojeda@xxxxxxxxxx>
---
rust/Makefile | 13 ++++++++++---
rust/bindgen_include_issue_workaround.h | 17 +++++++++++++++++
rust/bindgen_parameters | 19 +++++++++++++++++++
rust/bindings/bindings_helper.h | 3 +++
rust/helpers/helpers.c | 2 ++
rust/uapi/uapi_helper.h | 2 ++
6 files changed, 53 insertions(+), 3 deletions(-)
create mode 100644 rust/bindgen_include_issue_workaround.h

diff --git a/rust/Makefile b/rust/Makefile
index 9da9042fd627..3d3c1e2c0e6e 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -278,7 +278,11 @@ endif
# prototypes for functions like `memcpy` -- if this flag is not passed,
# `bindgen`-generated prototypes use `c_ulong` or `c_uint` depending on
# architecture instead of generating `usize`.
-bindgen_c_flags_final = $(bindgen_c_flags_lto) -fno-builtin -D__BINDGEN__
+bindgen_c_flags_final = $(filter-out -include \
+ %include/linux/compiler-version.h \
+ %include/linux/kconfig.h \
+ %include/linux/compiler_types.h \
+,$(bindgen_c_flags_lto)) -fno-builtin -D__BINDGEN__

quiet_cmd_bindgen = BINDGEN $@
cmd_bindgen = \
@@ -288,19 +292,22 @@ quiet_cmd_bindgen = BINDGEN $@
-o $@ -- $(bindgen_c_flags_final) -DMODULE \
$(bindgen_target_cflags) $(bindgen_target_extra)

+# TODO: proper workaround for `--clang-macro-fallback` interaction with `-Wp,-MMD,file.d`:
+#
+# https://github.com/rust-lang/rust-bindgen/issues/3070
$(obj)/bindings/bindings_generated.rs: private bindgen_target_flags = \
$(shell grep -Ev '^#|^$$' $(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)
+ $(call if_changed,bindgen)

$(obj)/uapi/uapi_generated.rs: private bindgen_target_flags = \
$(shell grep -Ev '^#|^$$' $(src)/bindgen_parameters)
$(obj)/uapi/uapi_generated.rs: $(src)/uapi/uapi_helper.h \
$(src)/bindgen_parameters FORCE
- $(call if_changed_dep,bindgen)
+ $(call if_changed,bindgen)

# See `CFLAGS_REMOVE_helpers.o` above. In addition, Clang on C does not warn
# with `-Wmissing-declarations` (unlike GCC), so it is not strictly needed here
diff --git a/rust/bindgen_include_issue_workaround.h b/rust/bindgen_include_issue_workaround.h
new file mode 100644
index 000000000000..d3ad51210732
--- /dev/null
+++ b/rust/bindgen_include_issue_workaround.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Workaround `bindgen`'s `--clang-macro-fallback` issue with `-include`:
+ *
+ * https://github.com/rust-lang/rust-bindgen/issues/3069
+ */
+
+#ifndef __BINDGEN_INCLUDE_ISSUE_WORKAROUND_H
+#define __BINDGEN_INCLUDE_ISSUE_WORKAROUND_H
+
+#ifdef __BINDGEN__
+#include <linux/compiler-version.h>
+#include <linux/kconfig.h>
+#include <linux/compiler_types.h>
+#endif /* __BINDGEN__ */
+
+#endif /* __BINDGEN_INCLUDE_ISSUE_WORKAROUND_H */
diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
index 0f96af8b9a7f..3452b88e1a1c 100644
--- a/rust/bindgen_parameters
+++ b/rust/bindgen_parameters
@@ -1,5 +1,24 @@
# SPDX-License-Identifier: GPL-2.0

+--clang-macro-fallback
+
+# Workaround for `--clang-macro-fallback` duplicate definitions:
+#
+# https://github.com/rust-lang/rust-bindgen/issues/3071
+--blocklist-item __TC_MQPRIO_MODE_MAX
+--blocklist-item __TC_MQPRIO_SHAPER_MAX
+--blocklist-item TCA_POLICE_RESULT
+--blocklist-item BPF_F_LINK
+--blocklist-item PAGE_SIZE
+--blocklist-item GFP_ATOMIC
+--blocklist-item GFP_KERNEL
+--blocklist-item GFP_KERNEL_ACCOUNT
+--blocklist-item GFP_NOWAIT
+--blocklist-item __GFP_ZERO
+--blocklist-item __GFP_HIGHMEM
+--blocklist-item __GFP_NOWARN
+--blocklist-item BLK_FEAT_ROTATIONAL
+
# We want to map these types to `isize`/`usize` manually, instead of
# define them as `int`/`long` depending on platform bitwidth.
--blocklist-type __kernel_s?size_t
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 5c4dfe22f41a..e98ba87efd4b 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -6,10 +6,13 @@
* Sorted alphabetically.
*/

+#include "../bindgen_include_issue_workaround.h"
+
#include <kunit/test.h>
#include <linux/blk-mq.h>
#include <linux/blk_types.h>
#include <linux/blkdev.h>
+#include <linux/cpufreq.h>
#include <linux/cred.h>
#include <linux/errname.h>
#include <linux/ethtool.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index dcf827a61b52..4048afe1008f 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -7,6 +7,8 @@
* Sorted alphabetically.
*/

+#include "../bindgen_include_issue_workaround.h"
+
#include "blk.c"
#include "bug.c"
#include "build_assert.c"
diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h
index 76d3f103e764..e54d4177d20e 100644
--- a/rust/uapi/uapi_helper.h
+++ b/rust/uapi/uapi_helper.h
@@ -6,6 +6,8 @@
* Sorted alphabetically.
*/

+#include "../bindgen_include_issue_workaround.h"
+
#include <uapi/asm-generic/ioctl.h>
#include <uapi/linux/mdio.h>
#include <uapi/linux/mii.h>
--
2.47.1

Next message: Guenter Roeck: "Re: [PATCH v2 1/3] hwmon: (acpi_power_meter) Fix uninitialized variables"
Previous message: Loic Poulain: "Re: [PATCH 1/2] bus: mhi: host: pci_generic: Use pci_try_reset_function() to avoid deadlock"
In reply to: Viresh Kumar: "Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]