rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arch: um: Don't rename vmap to kernel_vmap
@ 2025-11-22  8:32 David Gow
  2025-11-23 17:07 ` Miguel Ojeda
  0 siblings, 1 reply; 2+ messages in thread
From: David Gow @ 2025-11-22  8:32 UTC (permalink / raw)
  To: Richard Weinberger, Anton Ivanov, Johannes Berg, Miguel Ojeda,
	Paolo 'Blaisorblade' Giarrusso
  Cc: David Gow, Alice Ryhl, linux-um, linux-kernel, rust-for-linux

In order to work around the existence of a vmap symbol in libpcap, the
UML makefile unconditionally redefines vmap to kernel_vmap. However,
this not only affects the actual vmap symbol, but also anything else
named vmap, including a number of struct members in DRM.

This would not be too much of a problem, since all uses are also
updated, except we now have Rust DRM bindings, which expect the
corresponding Rust structs to have 'vmap' names. Since the redefinition
applies in bindgen, but not to Rust code, we end up with errors such as:

error[E0560]: struct `drm_gem_object_funcs` has no fields named `vmap`
  --> rust/kernel/drm/gem/mod.rs:210:9

Since, as far as I can tell, we no longer actually link to libpcap, it
should be safe to just remove this define unconditionally.

(If it's not, we can possibly either disable DRM Rust bindings under
UML, or move the redefinition of vmap behind some config option.)

We also take this opportunity to update the comment.

Signed-off-by: David Gow <davidgow@google.com>
---
 arch/um/Makefile | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/um/Makefile b/arch/um/Makefile
index 7be0143b5ba3..721b652ffb65 100644
--- a/arch/um/Makefile
+++ b/arch/um/Makefile
@@ -46,19 +46,17 @@ ARCH_INCLUDE	:= -I$(srctree)/$(SHARED_HEADERS)
 ARCH_INCLUDE	+= -I$(srctree)/$(HOST_DIR)/um/shared
 KBUILD_CPPFLAGS += -I$(srctree)/$(HOST_DIR)/um
 
-# -Dvmap=kernel_vmap prevents anything from referencing the libpcap.o symbol so
-# named - it's a common symbol in libpcap, so we get a binary which crashes.
-#
-# Same things for in6addr_loopback and mktime - found in libc. For these two we
-# only get link-time error, luckily.
+# -Dstrrchr=kernel_strrchr (as well as the various in6addr symbols) prevents
+#  anything from referencing
+# libc symbols with the same name, which can cause a linker error.
 #
 # -Dlongjmp=kernel_longjmp prevents anything from referencing the libpthread.a
 # embedded copy of longjmp, same thing for setjmp.
 #
-# These apply to USER_CFLAGS to.
+# These apply to USER_CFLAGS too.
 
 KBUILD_CFLAGS += $(CFLAGS) $(CFLAGS-y) -D__arch_um__ \
-	$(ARCH_INCLUDE) $(MODE_INCLUDE) -Dvmap=kernel_vmap	\
+	$(ARCH_INCLUDE) $(MODE_INCLUDE)	\
 	-Dlongjmp=kernel_longjmp -Dsetjmp=kernel_setjmp \
 	-Din6addr_loopback=kernel_in6addr_loopback \
 	-Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr \
-- 
2.52.0.rc2.455.g230fcf2819-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] arch: um: Don't rename vmap to kernel_vmap
  2025-11-22  8:32 [PATCH] arch: um: Don't rename vmap to kernel_vmap David Gow
@ 2025-11-23 17:07 ` Miguel Ojeda
  0 siblings, 0 replies; 2+ messages in thread
From: Miguel Ojeda @ 2025-11-23 17:07 UTC (permalink / raw)
  To: David Gow
  Cc: Richard Weinberger, Anton Ivanov, Johannes Berg, Miguel Ojeda,
	Paolo 'Blaisorblade' Giarrusso, Alice Ryhl, linux-um,
	linux-kernel, rust-for-linux

On Sat, Nov 22, 2025 at 9:32 AM David Gow <davidgow@google.com> wrote:
>
> In order to work around the existence of a vmap symbol in libpcap, the
> UML makefile unconditionally redefines vmap to kernel_vmap. However,
> this not only affects the actual vmap symbol, but also anything else
> named vmap, including a number of struct members in DRM.
>
> This would not be too much of a problem, since all uses are also
> updated, except we now have Rust DRM bindings, which expect the
> corresponding Rust structs to have 'vmap' names. Since the redefinition
> applies in bindgen, but not to Rust code, we end up with errors such as:
>
> error[E0560]: struct `drm_gem_object_funcs` has no fields named `vmap`
>   --> rust/kernel/drm/gem/mod.rs:210:9
>
> Since, as far as I can tell, we no longer actually link to libpcap, it
> should be safe to just remove this define unconditionally.
>
> (If it's not, we can possibly either disable DRM Rust bindings under
> UML, or move the redefinition of vmap behind some config option.)
>
> We also take this opportunity to update the comment.
>
> Signed-off-by: David Gow <davidgow@google.com>

Nice, thanks for this!

Yeah, I guess we would otherwise need to do the same kind of "wild"
macro replacement in Rust code to support this or conditional
compilation, and neither sounds good.

If it is not actually needed, then this sounds like a win-win.

It seems it was indeed gone in commit:

    12b8e7e69aa7 ("um: Remove obsolete pcap driver")

So it sounds reasonable to me assuming I am not missing anything,
which I may be... FWIW:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-11-23 17:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-22  8:32 [PATCH] arch: um: Don't rename vmap to kernel_vmap David Gow
2025-11-23 17:07 ` Miguel Ojeda

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).