* [PATCH v2] kbuild: rust: auto generate rust helper exports
@ 2024-08-17 16:51 Gary Guo
2024-08-18 0:47 ` Boqun Feng
2024-08-19 13:03 ` Miguel Ojeda
0 siblings, 2 replies; 9+ messages in thread
From: Gary Guo @ 2024-08-17 16:51 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Dirk Behme
Cc: rust-for-linux, linux-kernel
This removes the need for explicit exporting all symbols.
Generate helper exports similar to what's currently done for Rust
crates. These helpers are exclusively called from within Rust code
and therefore can be treated similar as other Rust symbols.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
Notes:
Changes in v2:
- Split from the helper inline RFC [1]
- Rebased on Andreas' split helper patch [2]
- Moved comments about helper export to Makefile
[1]: https://lore.kernel.org/rust-for-linux/20240529202817.3641974-2-gary@garyguo.net/
[2]: https://lore.kernel.org/rust-for-linux/20240815103016.2771842-1-nmi@metaspace.dk/
rust/Makefile | 16 ++++++++++++++--
rust/exports.c | 1 +
rust/helpers/README.md | 12 ------------
rust/helpers/blk.c | 2 --
rust/helpers/bug.c | 1 -
rust/helpers/build_bug.c | 1 -
rust/helpers/err.c | 3 ---
rust/helpers/kunit.c | 1 -
rust/helpers/mutex.c | 1 -
rust/helpers/page.c | 5 -----
rust/helpers/refcount.c | 3 ---
rust/helpers/signal.c | 1 -
rust/helpers/slab.c | 1 -
rust/helpers/spinlock.c | 3 ---
rust/helpers/task.c | 3 ---
rust/helpers/uaccess.c | 2 --
rust/helpers/wait.c | 1 -
rust/helpers/workqueue.c | 1 -
18 files changed, 15 insertions(+), 43 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index 967da95ba8b0..3171a9021494 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -16,8 +16,8 @@ no-clean-files += libmacros.so
always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o
-always-$(CONFIG_RUST) += exports_alloc_generated.h exports_bindings_generated.h \
- exports_kernel_generated.h
+always-$(CONFIG_RUST) += exports_alloc_generated.h exports_helpers_generated.h \
+ exports_bindings_generated.h exports_kernel_generated.h
always-$(CONFIG_RUST) += uapi/uapi_generated.rs
obj-$(CONFIG_RUST) += uapi.o
@@ -313,6 +313,18 @@ $(obj)/exports_core_generated.h: $(obj)/core.o FORCE
$(obj)/exports_alloc_generated.h: $(obj)/alloc.o FORCE
$(call if_changed,exports)
+# Even though Rust kernel modules should never use the bindings directly,
+# symbols from bindings crate and the C helpers need to be exported because
+# Rust generics and inlined functions may not get their code generated in the
+# crate where they are defined. Other helpers, called from non-inline
+# functions, may not be exported, in principle. However, in general, the Rust
+# compiler does not guarantee codegen will be performed for a non-inline
+# function either. Therefore, we export all symbols from helpers and bindings.
+# In the future, this may be revisited to reduce the number of exports after
+# the compiler is informed about the places codegen is required.
+$(obj)/exports_helpers_generated.h: $(obj)/helpers.o FORCE
+ $(call if_changed,exports)
+
$(obj)/exports_bindings_generated.h: $(obj)/bindings.o FORCE
$(call if_changed,exports)
diff --git a/rust/exports.c b/rust/exports.c
index 3803c21d1403..e5695f3b45b7 100644
--- a/rust/exports.c
+++ b/rust/exports.c
@@ -17,6 +17,7 @@
#include "exports_core_generated.h"
#include "exports_alloc_generated.h"
+#include "exports_helpers_generated.h"
#include "exports_bindings_generated.h"
#include "exports_kernel_generated.h"
diff --git a/rust/helpers/README.md b/rust/helpers/README.md
index 940b24bb552d..f2be545ef946 100644
--- a/rust/helpers/README.md
+++ b/rust/helpers/README.md
@@ -3,15 +3,3 @@
Non-trivial C macros cannot be used in Rust. Similarly, inlined C functions
cannot be called either. The files in this directory explicitly create functions
("helpers") that wrap those so that they can be called from Rust.
-
-Even though Rust kernel modules should never use the bindings directly, some of
-these helpers need to be exported because Rust generics and inlined functions
-may not get their code generated in the crate where they are defined. Other
-helpers, called from non-inline functions, may not be exported, in principle.
-However, in general, the Rust compiler does not guarantee codegen will be
-performed for a non-inline function either. Therefore, this file exports all the
-helpers. In the future, this may be revisited to reduce the number of exports
-after the compiler is informed about the places codegen is required.
-
-All symbols are exported as GPL-only to guarantee no GPL-only feature is
-accidentally exposed.
diff --git a/rust/helpers/blk.c b/rust/helpers/blk.c
index d99c965eb59b..cc9f4e6a2d23 100644
--- a/rust/helpers/blk.c
+++ b/rust/helpers/blk.c
@@ -7,10 +7,8 @@ void *rust_helper_blk_mq_rq_to_pdu(struct request *rq)
{
return blk_mq_rq_to_pdu(rq);
}
-EXPORT_SYMBOL_GPL(rust_helper_blk_mq_rq_to_pdu);
struct request *rust_helper_blk_mq_rq_from_pdu(void *pdu)
{
return blk_mq_rq_from_pdu(pdu);
}
-EXPORT_SYMBOL_GPL(rust_helper_blk_mq_rq_from_pdu);
diff --git a/rust/helpers/bug.c b/rust/helpers/bug.c
index e2afbad23dcd..e2d13babc737 100644
--- a/rust/helpers/bug.c
+++ b/rust/helpers/bug.c
@@ -6,4 +6,3 @@ __noreturn void rust_helper_BUG(void)
{
BUG();
}
-EXPORT_SYMBOL_GPL(rust_helper_BUG);
diff --git a/rust/helpers/build_bug.c b/rust/helpers/build_bug.c
index f3106f248485..e994f7b5928c 100644
--- a/rust/helpers/build_bug.c
+++ b/rust/helpers/build_bug.c
@@ -7,4 +7,3 @@ const char *rust_helper_errname(int err)
{
return errname(err);
}
-EXPORT_SYMBOL_GPL(rust_helper_errname);
diff --git a/rust/helpers/err.c b/rust/helpers/err.c
index fba4e0be64f5..be3d45ef78a2 100644
--- a/rust/helpers/err.c
+++ b/rust/helpers/err.c
@@ -7,16 +7,13 @@ __force void *rust_helper_ERR_PTR(long err)
{
return ERR_PTR(err);
}
-EXPORT_SYMBOL_GPL(rust_helper_ERR_PTR);
bool rust_helper_IS_ERR(__force const void *ptr)
{
return IS_ERR(ptr);
}
-EXPORT_SYMBOL_GPL(rust_helper_IS_ERR);
long rust_helper_PTR_ERR(__force const void *ptr)
{
return PTR_ERR(ptr);
}
-EXPORT_SYMBOL_GPL(rust_helper_PTR_ERR);
diff --git a/rust/helpers/kunit.c b/rust/helpers/kunit.c
index 905e4ff4424a..9d725067eb3b 100644
--- a/rust/helpers/kunit.c
+++ b/rust/helpers/kunit.c
@@ -7,4 +7,3 @@ struct kunit *rust_helper_kunit_get_current_test(void)
{
return kunit_get_current_test();
}
-EXPORT_SYMBOL_GPL(rust_helper_kunit_get_current_test);
diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c
index 29fd141c387d..200db7e6279f 100644
--- a/rust/helpers/mutex.c
+++ b/rust/helpers/mutex.c
@@ -7,4 +7,3 @@ void rust_helper_mutex_lock(struct mutex *lock)
{
mutex_lock(lock);
}
-EXPORT_SYMBOL_GPL(rust_helper_mutex_lock);
diff --git a/rust/helpers/page.c b/rust/helpers/page.c
index b3280c80b283..b3f2b8fbf87f 100644
--- a/rust/helpers/page.c
+++ b/rust/helpers/page.c
@@ -7,18 +7,13 @@ struct page *rust_helper_alloc_pages(gfp_t gfp_mask, unsigned int order)
{
return alloc_pages(gfp_mask, order);
}
-EXPORT_SYMBOL_GPL(rust_helper_alloc_pages);
void *rust_helper_kmap_local_page(struct page *page)
{
return kmap_local_page(page);
}
-EXPORT_SYMBOL_GPL(rust_helper_kmap_local_page);
void rust_helper_kunmap_local(const void *addr)
{
kunmap_local(addr);
}
-EXPORT_SYMBOL_GPL(rust_helper_kunmap_local);
-
-
diff --git a/rust/helpers/refcount.c b/rust/helpers/refcount.c
index 13ab64805f77..f47afc148ec3 100644
--- a/rust/helpers/refcount.c
+++ b/rust/helpers/refcount.c
@@ -7,16 +7,13 @@ refcount_t rust_helper_REFCOUNT_INIT(int n)
{
return (refcount_t)REFCOUNT_INIT(n);
}
-EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
void rust_helper_refcount_inc(refcount_t *r)
{
refcount_inc(r);
}
-EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
bool rust_helper_refcount_dec_and_test(refcount_t *r)
{
return refcount_dec_and_test(r);
}
-EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
diff --git a/rust/helpers/signal.c b/rust/helpers/signal.c
index d44e8096b8a9..63c407f80c26 100644
--- a/rust/helpers/signal.c
+++ b/rust/helpers/signal.c
@@ -7,4 +7,3 @@ int rust_helper_signal_pending(struct task_struct *t)
{
return signal_pending(t);
}
-EXPORT_SYMBOL_GPL(rust_helper_signal_pending);
diff --git a/rust/helpers/slab.c b/rust/helpers/slab.c
index 3e0a1a173d8a..f043e087f9d6 100644
--- a/rust/helpers/slab.c
+++ b/rust/helpers/slab.c
@@ -7,4 +7,3 @@ rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
{
return krealloc(objp, new_size, flags);
}
-EXPORT_SYMBOL_GPL(rust_helper_krealloc);
diff --git a/rust/helpers/spinlock.c b/rust/helpers/spinlock.c
index 04fd8ddb4986..acc1376b833c 100644
--- a/rust/helpers/spinlock.c
+++ b/rust/helpers/spinlock.c
@@ -12,16 +12,13 @@ void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
spin_lock_init(lock);
#endif
}
-EXPORT_SYMBOL_GPL(rust_helper___spin_lock_init);
void rust_helper_spin_lock(spinlock_t *lock)
{
spin_lock(lock);
}
-EXPORT_SYMBOL_GPL(rust_helper_spin_lock);
void rust_helper_spin_unlock(spinlock_t *lock)
{
spin_unlock(lock);
}
-EXPORT_SYMBOL_GPL(rust_helper_spin_unlock);
diff --git a/rust/helpers/task.c b/rust/helpers/task.c
index b176c347f0d4..7ac789232d11 100644
--- a/rust/helpers/task.c
+++ b/rust/helpers/task.c
@@ -7,16 +7,13 @@ struct task_struct *rust_helper_get_current(void)
{
return current;
}
-EXPORT_SYMBOL_GPL(rust_helper_get_current);
void rust_helper_get_task_struct(struct task_struct *t)
{
get_task_struct(t);
}
-EXPORT_SYMBOL_GPL(rust_helper_get_task_struct);
void rust_helper_put_task_struct(struct task_struct *t)
{
put_task_struct(t);
}
-EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
diff --git a/rust/helpers/uaccess.c b/rust/helpers/uaccess.c
index 3d004ac1c180..f49076f813cd 100644
--- a/rust/helpers/uaccess.c
+++ b/rust/helpers/uaccess.c
@@ -7,11 +7,9 @@ unsigned long rust_helper_copy_from_user(void *to, const void __user *from,
{
return copy_from_user(to, from, n);
}
-EXPORT_SYMBOL_GPL(rust_helper_copy_from_user);
unsigned long rust_helper_copy_to_user(void __user *to, const void *from,
unsigned long n)
{
return copy_to_user(to, from, n);
}
-EXPORT_SYMBOL_GPL(rust_helper_copy_to_user);
diff --git a/rust/helpers/wait.c b/rust/helpers/wait.c
index bf361f40c7cb..c7336bbf2750 100644
--- a/rust/helpers/wait.c
+++ b/rust/helpers/wait.c
@@ -7,4 +7,3 @@ void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
{
init_wait(wq_entry);
}
-EXPORT_SYMBOL_GPL(rust_helper_init_wait);
diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c
index 12e2ee66aa4f..f59427acc323 100644
--- a/rust/helpers/workqueue.c
+++ b/rust/helpers/workqueue.c
@@ -13,4 +13,3 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
INIT_LIST_HEAD(&work->entry);
work->func = func;
}
-EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key);
base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
prerequisite-patch-id: 97916a5ce500a204e6e5b4c5726691f080d6fbcc
--
2.44.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2] kbuild: rust: auto generate rust helper exports
2024-08-17 16:51 [PATCH v2] kbuild: rust: auto generate rust helper exports Gary Guo
@ 2024-08-18 0:47 ` Boqun Feng
2024-08-18 23:40 ` Miguel Ojeda
2024-08-19 13:03 ` Miguel Ojeda
1 sibling, 1 reply; 9+ messages in thread
From: Boqun Feng @ 2024-08-18 0:47 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Dirk Behme, rust-for-linux, linux-kernel
On Sat, Aug 17, 2024 at 05:51:32PM +0100, Gary Guo wrote:
> This removes the need for explicit exporting all symbols.
>
> Generate helper exports similar to what's currently done for Rust
> crates. These helpers are exclusively called from within Rust code
> and therefore can be treated similar as other Rust symbols.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
This also passed my test script (x86, arm64 and riscv build and kunit
tests).
Miguel, I think it makes more sense if we take this one and the
helpers.c split one as early as possible, given they are treewide
changes ;-)
Regards,
Boqun
> ---
>
> Notes:
> Changes in v2:
> - Split from the helper inline RFC [1]
> - Rebased on Andreas' split helper patch [2]
> - Moved comments about helper export to Makefile
>
> [1]: https://lore.kernel.org/rust-for-linux/20240529202817.3641974-2-gary@garyguo.net/
> [2]: https://lore.kernel.org/rust-for-linux/20240815103016.2771842-1-nmi@metaspace.dk/
>
> rust/Makefile | 16 ++++++++++++++--
> rust/exports.c | 1 +
> rust/helpers/README.md | 12 ------------
> rust/helpers/blk.c | 2 --
> rust/helpers/bug.c | 1 -
> rust/helpers/build_bug.c | 1 -
> rust/helpers/err.c | 3 ---
> rust/helpers/kunit.c | 1 -
> rust/helpers/mutex.c | 1 -
> rust/helpers/page.c | 5 -----
> rust/helpers/refcount.c | 3 ---
> rust/helpers/signal.c | 1 -
> rust/helpers/slab.c | 1 -
> rust/helpers/spinlock.c | 3 ---
> rust/helpers/task.c | 3 ---
> rust/helpers/uaccess.c | 2 --
> rust/helpers/wait.c | 1 -
> rust/helpers/workqueue.c | 1 -
> 18 files changed, 15 insertions(+), 43 deletions(-)
>
> diff --git a/rust/Makefile b/rust/Makefile
> index 967da95ba8b0..3171a9021494 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -16,8 +16,8 @@ no-clean-files += libmacros.so
>
> always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
> obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o
> -always-$(CONFIG_RUST) += exports_alloc_generated.h exports_bindings_generated.h \
> - exports_kernel_generated.h
> +always-$(CONFIG_RUST) += exports_alloc_generated.h exports_helpers_generated.h \
> + exports_bindings_generated.h exports_kernel_generated.h
>
> always-$(CONFIG_RUST) += uapi/uapi_generated.rs
> obj-$(CONFIG_RUST) += uapi.o
> @@ -313,6 +313,18 @@ $(obj)/exports_core_generated.h: $(obj)/core.o FORCE
> $(obj)/exports_alloc_generated.h: $(obj)/alloc.o FORCE
> $(call if_changed,exports)
>
> +# Even though Rust kernel modules should never use the bindings directly,
> +# symbols from bindings crate and the C helpers need to be exported because
> +# Rust generics and inlined functions may not get their code generated in the
> +# crate where they are defined. Other helpers, called from non-inline
> +# functions, may not be exported, in principle. However, in general, the Rust
> +# compiler does not guarantee codegen will be performed for a non-inline
> +# function either. Therefore, we export all symbols from helpers and bindings.
> +# In the future, this may be revisited to reduce the number of exports after
> +# the compiler is informed about the places codegen is required.
> +$(obj)/exports_helpers_generated.h: $(obj)/helpers.o FORCE
> + $(call if_changed,exports)
> +
> $(obj)/exports_bindings_generated.h: $(obj)/bindings.o FORCE
> $(call if_changed,exports)
>
> diff --git a/rust/exports.c b/rust/exports.c
> index 3803c21d1403..e5695f3b45b7 100644
> --- a/rust/exports.c
> +++ b/rust/exports.c
> @@ -17,6 +17,7 @@
>
> #include "exports_core_generated.h"
> #include "exports_alloc_generated.h"
> +#include "exports_helpers_generated.h"
> #include "exports_bindings_generated.h"
> #include "exports_kernel_generated.h"
>
> diff --git a/rust/helpers/README.md b/rust/helpers/README.md
> index 940b24bb552d..f2be545ef946 100644
> --- a/rust/helpers/README.md
> +++ b/rust/helpers/README.md
> @@ -3,15 +3,3 @@
> Non-trivial C macros cannot be used in Rust. Similarly, inlined C functions
> cannot be called either. The files in this directory explicitly create functions
> ("helpers") that wrap those so that they can be called from Rust.
> -
> -Even though Rust kernel modules should never use the bindings directly, some of
> -these helpers need to be exported because Rust generics and inlined functions
> -may not get their code generated in the crate where they are defined. Other
> -helpers, called from non-inline functions, may not be exported, in principle.
> -However, in general, the Rust compiler does not guarantee codegen will be
> -performed for a non-inline function either. Therefore, this file exports all the
> -helpers. In the future, this may be revisited to reduce the number of exports
> -after the compiler is informed about the places codegen is required.
> -
> -All symbols are exported as GPL-only to guarantee no GPL-only feature is
> -accidentally exposed.
> diff --git a/rust/helpers/blk.c b/rust/helpers/blk.c
> index d99c965eb59b..cc9f4e6a2d23 100644
> --- a/rust/helpers/blk.c
> +++ b/rust/helpers/blk.c
> @@ -7,10 +7,8 @@ void *rust_helper_blk_mq_rq_to_pdu(struct request *rq)
> {
> return blk_mq_rq_to_pdu(rq);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_blk_mq_rq_to_pdu);
>
> struct request *rust_helper_blk_mq_rq_from_pdu(void *pdu)
> {
> return blk_mq_rq_from_pdu(pdu);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_blk_mq_rq_from_pdu);
> diff --git a/rust/helpers/bug.c b/rust/helpers/bug.c
> index e2afbad23dcd..e2d13babc737 100644
> --- a/rust/helpers/bug.c
> +++ b/rust/helpers/bug.c
> @@ -6,4 +6,3 @@ __noreturn void rust_helper_BUG(void)
> {
> BUG();
> }
> -EXPORT_SYMBOL_GPL(rust_helper_BUG);
> diff --git a/rust/helpers/build_bug.c b/rust/helpers/build_bug.c
> index f3106f248485..e994f7b5928c 100644
> --- a/rust/helpers/build_bug.c
> +++ b/rust/helpers/build_bug.c
> @@ -7,4 +7,3 @@ const char *rust_helper_errname(int err)
> {
> return errname(err);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_errname);
> diff --git a/rust/helpers/err.c b/rust/helpers/err.c
> index fba4e0be64f5..be3d45ef78a2 100644
> --- a/rust/helpers/err.c
> +++ b/rust/helpers/err.c
> @@ -7,16 +7,13 @@ __force void *rust_helper_ERR_PTR(long err)
> {
> return ERR_PTR(err);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_ERR_PTR);
>
> bool rust_helper_IS_ERR(__force const void *ptr)
> {
> return IS_ERR(ptr);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_IS_ERR);
>
> long rust_helper_PTR_ERR(__force const void *ptr)
> {
> return PTR_ERR(ptr);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_PTR_ERR);
> diff --git a/rust/helpers/kunit.c b/rust/helpers/kunit.c
> index 905e4ff4424a..9d725067eb3b 100644
> --- a/rust/helpers/kunit.c
> +++ b/rust/helpers/kunit.c
> @@ -7,4 +7,3 @@ struct kunit *rust_helper_kunit_get_current_test(void)
> {
> return kunit_get_current_test();
> }
> -EXPORT_SYMBOL_GPL(rust_helper_kunit_get_current_test);
> diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c
> index 29fd141c387d..200db7e6279f 100644
> --- a/rust/helpers/mutex.c
> +++ b/rust/helpers/mutex.c
> @@ -7,4 +7,3 @@ void rust_helper_mutex_lock(struct mutex *lock)
> {
> mutex_lock(lock);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_mutex_lock);
> diff --git a/rust/helpers/page.c b/rust/helpers/page.c
> index b3280c80b283..b3f2b8fbf87f 100644
> --- a/rust/helpers/page.c
> +++ b/rust/helpers/page.c
> @@ -7,18 +7,13 @@ struct page *rust_helper_alloc_pages(gfp_t gfp_mask, unsigned int order)
> {
> return alloc_pages(gfp_mask, order);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_alloc_pages);
>
> void *rust_helper_kmap_local_page(struct page *page)
> {
> return kmap_local_page(page);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_kmap_local_page);
>
> void rust_helper_kunmap_local(const void *addr)
> {
> kunmap_local(addr);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_kunmap_local);
> -
> -
> diff --git a/rust/helpers/refcount.c b/rust/helpers/refcount.c
> index 13ab64805f77..f47afc148ec3 100644
> --- a/rust/helpers/refcount.c
> +++ b/rust/helpers/refcount.c
> @@ -7,16 +7,13 @@ refcount_t rust_helper_REFCOUNT_INIT(int n)
> {
> return (refcount_t)REFCOUNT_INIT(n);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
>
> void rust_helper_refcount_inc(refcount_t *r)
> {
> refcount_inc(r);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
>
> bool rust_helper_refcount_dec_and_test(refcount_t *r)
> {
> return refcount_dec_and_test(r);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> diff --git a/rust/helpers/signal.c b/rust/helpers/signal.c
> index d44e8096b8a9..63c407f80c26 100644
> --- a/rust/helpers/signal.c
> +++ b/rust/helpers/signal.c
> @@ -7,4 +7,3 @@ int rust_helper_signal_pending(struct task_struct *t)
> {
> return signal_pending(t);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_signal_pending);
> diff --git a/rust/helpers/slab.c b/rust/helpers/slab.c
> index 3e0a1a173d8a..f043e087f9d6 100644
> --- a/rust/helpers/slab.c
> +++ b/rust/helpers/slab.c
> @@ -7,4 +7,3 @@ rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
> {
> return krealloc(objp, new_size, flags);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_krealloc);
> diff --git a/rust/helpers/spinlock.c b/rust/helpers/spinlock.c
> index 04fd8ddb4986..acc1376b833c 100644
> --- a/rust/helpers/spinlock.c
> +++ b/rust/helpers/spinlock.c
> @@ -12,16 +12,13 @@ void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
> spin_lock_init(lock);
> #endif
> }
> -EXPORT_SYMBOL_GPL(rust_helper___spin_lock_init);
>
> void rust_helper_spin_lock(spinlock_t *lock)
> {
> spin_lock(lock);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_spin_lock);
>
> void rust_helper_spin_unlock(spinlock_t *lock)
> {
> spin_unlock(lock);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_spin_unlock);
> diff --git a/rust/helpers/task.c b/rust/helpers/task.c
> index b176c347f0d4..7ac789232d11 100644
> --- a/rust/helpers/task.c
> +++ b/rust/helpers/task.c
> @@ -7,16 +7,13 @@ struct task_struct *rust_helper_get_current(void)
> {
> return current;
> }
> -EXPORT_SYMBOL_GPL(rust_helper_get_current);
>
> void rust_helper_get_task_struct(struct task_struct *t)
> {
> get_task_struct(t);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_get_task_struct);
>
> void rust_helper_put_task_struct(struct task_struct *t)
> {
> put_task_struct(t);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
> diff --git a/rust/helpers/uaccess.c b/rust/helpers/uaccess.c
> index 3d004ac1c180..f49076f813cd 100644
> --- a/rust/helpers/uaccess.c
> +++ b/rust/helpers/uaccess.c
> @@ -7,11 +7,9 @@ unsigned long rust_helper_copy_from_user(void *to, const void __user *from,
> {
> return copy_from_user(to, from, n);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_copy_from_user);
>
> unsigned long rust_helper_copy_to_user(void __user *to, const void *from,
> unsigned long n)
> {
> return copy_to_user(to, from, n);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_copy_to_user);
> diff --git a/rust/helpers/wait.c b/rust/helpers/wait.c
> index bf361f40c7cb..c7336bbf2750 100644
> --- a/rust/helpers/wait.c
> +++ b/rust/helpers/wait.c
> @@ -7,4 +7,3 @@ void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
> {
> init_wait(wq_entry);
> }
> -EXPORT_SYMBOL_GPL(rust_helper_init_wait);
> diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c
> index 12e2ee66aa4f..f59427acc323 100644
> --- a/rust/helpers/workqueue.c
> +++ b/rust/helpers/workqueue.c
> @@ -13,4 +13,3 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
> INIT_LIST_HEAD(&work->entry);
> work->func = func;
> }
> -EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key);
>
> base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
> prerequisite-patch-id: 97916a5ce500a204e6e5b4c5726691f080d6fbcc
> --
> 2.44.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] kbuild: rust: auto generate rust helper exports
2024-08-18 0:47 ` Boqun Feng
@ 2024-08-18 23:40 ` Miguel Ojeda
2024-08-18 23:56 ` Gary Guo
2024-08-19 0:57 ` Boqun Feng
0 siblings, 2 replies; 9+ messages in thread
From: Miguel Ojeda @ 2024-08-18 23:40 UTC (permalink / raw)
To: Boqun Feng
Cc: Gary Guo, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Dirk Behme, rust-for-linux, linux-kernel
On Sun, Aug 18, 2024 at 2:49 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>
> This also passed my test script (x86, arm64 and riscv build and kunit
> tests).
Hmm... I think you either tested another one, or you manually fixed
the dependency (`$(obj)/helpers/helpers.o`).
Or am I confused?
> Miguel, I think it makes more sense if we take this one and the
> helpers.c split one as early as possible, given they are treewide
> changes ;-)
Agreed, we should take it soon. I have rebased it on top of
`rust-next` and fixed the dependency.
Please take a look:
https://github.com/Rust-for-Linux/linux/commit/0d6e3e8d3677ee7b5cccf9bc002e18f140c02a4e
I will put it in after we confirm we are not confused :) (and after we
get a linux-next round or two for what is currently there).
Extra `Tested-by`s would be nice!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] kbuild: rust: auto generate rust helper exports
2024-08-18 23:40 ` Miguel Ojeda
@ 2024-08-18 23:56 ` Gary Guo
2024-08-19 0:17 ` Miguel Ojeda
2024-08-19 0:57 ` Boqun Feng
1 sibling, 1 reply; 9+ messages in thread
From: Gary Guo @ 2024-08-18 23:56 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Dirk Behme, rust-for-linux, linux-kernel
On Mon, 19 Aug 2024 01:40:44 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> On Sun, Aug 18, 2024 at 2:49 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> >
> > This also passed my test script (x86, arm64 and riscv build and kunit
> > tests).
>
> Hmm... I think you either tested another one, or you manually fixed
> the dependency (`$(obj)/helpers/helpers.o`).
>
> Or am I confused?
>
> > Miguel, I think it makes more sense if we take this one and the
> > helpers.c split one as early as possible, given they are treewide
> > changes ;-)
>
> Agreed, we should take it soon. I have rebased it on top of
> `rust-next` and fixed the dependency.
>
> Please take a look:
>
> https://github.com/Rust-for-Linux/linux/commit/0d6e3e8d3677ee7b5cccf9bc002e18f140c02a4e
>
> I will put it in after we confirm we are not confused :) (and after we
> get a linux-next round or two for what is currently there).
>
> Extra `Tested-by`s would be nice!
>
> Cheers,
> Miguel
Hi Miguel,
It's the mistake on my patch. I noticed the exact issue while I was
trying to rebase my helper-lto patch :)
I believe that reason that it builds fine for me previously and for
Boqun was that we didn't delete the helpers.o generated prior to
applying the helper split patch, and we know that kbuild is not
hermetic.
Your rebase and the fix is identical to the one I got locally, and I
can confirm that it compiles fine for a clean build.
Best,
Gary
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] kbuild: rust: auto generate rust helper exports
2024-08-18 23:56 ` Gary Guo
@ 2024-08-19 0:17 ` Miguel Ojeda
2024-08-19 0:27 ` Boqun Feng
0 siblings, 1 reply; 9+ messages in thread
From: Miguel Ojeda @ 2024-08-19 0:17 UTC (permalink / raw)
To: Gary Guo
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Dirk Behme, rust-for-linux, linux-kernel
On Mon, Aug 19, 2024 at 1:56 AM Gary Guo <gary@garyguo.net> wrote:
>
> It's the mistake on my patch. I noticed the exact issue while I was
> trying to rebase my helper-lto patch :)
>
> I believe that reason that it builds fine for me previously and for
> Boqun was that we didn't delete the helpers.o generated prior to
> applying the helper split patch, and we know that kbuild is not
> hermetic.
Yeah, probably it was that -- as you say, it would require a stale
`rust/helpers.o` from a build before Andreas' patch (and not just
before this patch).
> Your rebase and the fix is identical to the one I got locally, and I
> can confirm that it compiles fine for a clean build.
Thanks a lot for taking a look!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] kbuild: rust: auto generate rust helper exports
2024-08-19 0:17 ` Miguel Ojeda
@ 2024-08-19 0:27 ` Boqun Feng
0 siblings, 0 replies; 9+ messages in thread
From: Boqun Feng @ 2024-08-19 0:27 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Gary Guo, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Dirk Behme, rust-for-linux, linux-kernel
On Mon, Aug 19, 2024 at 02:17:08AM +0200, Miguel Ojeda wrote:
> On Mon, Aug 19, 2024 at 1:56 AM Gary Guo <gary@garyguo.net> wrote:
> >
> > It's the mistake on my patch. I noticed the exact issue while I was
> > trying to rebase my helper-lto patch :)
> >
> > I believe that reason that it builds fine for me previously and for
> > Boqun was that we didn't delete the helpers.o generated prior to
> > applying the helper split patch, and we know that kbuild is not
> > hermetic.
>
Oh yes, that's the case, I have a .kunit/rust/helpers.o created at Aug 7
:-(
> Yeah, probably it was that -- as you say, it would require a stale
> `rust/helpers.o` from a build before Andreas' patch (and not just
> before this patch).
>
> > Your rebase and the fix is identical to the one I got locally, and I
> > can confirm that it compiles fine for a clean build.
>
> Thanks a lot for taking a look!
>
Let me rerun your fixed version.
Regards,
Boqun
> Cheers,
> Miguel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] kbuild: rust: auto generate rust helper exports
2024-08-18 23:40 ` Miguel Ojeda
2024-08-18 23:56 ` Gary Guo
@ 2024-08-19 0:57 ` Boqun Feng
1 sibling, 0 replies; 9+ messages in thread
From: Boqun Feng @ 2024-08-19 0:57 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Gary Guo, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Dirk Behme, rust-for-linux, linux-kernel
On Mon, Aug 19, 2024 at 01:40:44AM +0200, Miguel Ojeda wrote:
> On Sun, Aug 18, 2024 at 2:49 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> >
> > This also passed my test script (x86, arm64 and riscv build and kunit
> > tests).
>
> Hmm... I think you either tested another one, or you manually fixed
> the dependency (`$(obj)/helpers/helpers.o`).
>
> Or am I confused?
>
> > Miguel, I think it makes more sense if we take this one and the
> > helpers.c split one as early as possible, given they are treewide
> > changes ;-)
>
> Agreed, we should take it soon. I have rebased it on top of
> `rust-next` and fixed the dependency.
>
> Please take a look:
>
> https://github.com/Rust-for-Linux/linux/commit/0d6e3e8d3677ee7b5cccf9bc002e18f140c02a4e
>
> I will put it in after we confirm we are not confused :) (and after we
> get a linux-next round or two for what is currently there).
>
> Extra `Tested-by`s would be nice!
>
I cleaned my local .kunit cache and re-run all the tests, they all
passed.
Tested-by: Boqun Feng <boqun.feng@gmail.com>
Regards,
Boqun
> Cheers,
> Miguel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] kbuild: rust: auto generate rust helper exports
2024-08-17 16:51 [PATCH v2] kbuild: rust: auto generate rust helper exports Gary Guo
2024-08-18 0:47 ` Boqun Feng
@ 2024-08-19 13:03 ` Miguel Ojeda
2024-09-25 20:49 ` Sami Tolvanen
1 sibling, 1 reply; 9+ messages in thread
From: Miguel Ojeda @ 2024-08-19 13:03 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Dirk Behme, rust-for-linux, linux-kernel
On Sat, Aug 17, 2024 at 6:54 PM Gary Guo <gary@garyguo.net> wrote:
>
> This removes the need for explicit exporting all symbols.
>
> Generate helper exports similar to what's currently done for Rust
> crates. These helpers are exclusively called from within Rust code
> and therefore can be treated similar as other Rust symbols.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
Applied to `rust-next` -- thanks everyone!
[ Fixed dependency path, reworded slightly, edited comment a bit and
rebased on top of the changes made when applying Andreas' patch
(e.g. no `README.md` anymore, so moved the edits). - Miguel ]
Cheers,
Miguel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] kbuild: rust: auto generate rust helper exports
2024-08-19 13:03 ` Miguel Ojeda
@ 2024-09-25 20:49 ` Sami Tolvanen
0 siblings, 0 replies; 9+ messages in thread
From: Sami Tolvanen @ 2024-09-25 20:49 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Gary Guo, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Dirk Behme, rust-for-linux, linux-kernel
Hi folks,
On Mon, Aug 19, 2024 at 1:04 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Aug 17, 2024 at 6:54 PM Gary Guo <gary@garyguo.net> wrote:
> >
> > This removes the need for explicit exporting all symbols.
> >
> > Generate helper exports similar to what's currently done for Rust
> > crates. These helpers are exclusively called from within Rust code
> > and therefore can be treated similar as other Rust symbols.
> >
> > Signed-off-by: Gary Guo <gary@garyguo.net>
>
> Applied to `rust-next` -- thanks everyone!
>
> [ Fixed dependency path, reworded slightly, edited comment a bit and
> rebased on top of the changes made when applying Andreas' patch
> (e.g. no `README.md` anymore, so moved the edits). - Miguel ]
I noticed this patch when I was rebasing the modversions series, and
it breaks module versioning for the C helpers because they no longer
export anything. Since helpers.o is not Rust code either, it seems
we'd have to add a special rule to generate modversions just for this
one object file, which doesn't feel ideal.
I ended up reverting this patch for now in my personal test tree [1],
but I'd like to understand if there are benefits in using Rust-style
exports for this C code other than being able to drop a few lines of
exports? The comment added to rust/Makefile says that "In the future,
this may be revisited to reduce the number of exports" which sounds
like this might be a temporary clean-up?
[1] https://github.com/samitolvanen/linux/commits/rustmodversions-v3
Sami
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-09-25 20:49 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-17 16:51 [PATCH v2] kbuild: rust: auto generate rust helper exports Gary Guo
2024-08-18 0:47 ` Boqun Feng
2024-08-18 23:40 ` Miguel Ojeda
2024-08-18 23:56 ` Gary Guo
2024-08-19 0:17 ` Miguel Ojeda
2024-08-19 0:27 ` Boqun Feng
2024-08-19 0:57 ` Boqun Feng
2024-08-19 13:03 ` Miguel Ojeda
2024-09-25 20:49 ` Sami Tolvanen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).