* [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; 6+ 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] 6+ 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 2025-11-24 7:49 ` Johannes Berg 0 siblings, 1 reply; 6+ 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] 6+ messages in thread
* Re: [PATCH] arch: um: Don't rename vmap to kernel_vmap 2025-11-23 17:07 ` Miguel Ojeda @ 2025-11-24 7:49 ` Johannes Berg 2025-11-25 7:36 ` David Gow 0 siblings, 1 reply; 6+ messages in thread From: Johannes Berg @ 2025-11-24 7:49 UTC (permalink / raw) To: Miguel Ojeda, David Gow Cc: Richard Weinberger, Anton Ivanov, Miguel Ojeda, Paolo 'Blaisorblade' Giarrusso, Alice Ryhl, linux-um, linux-kernel, rust-for-linux On Sun, 2025-11-23 at 18:07 +0100, Miguel Ojeda wrote: > 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") Indeed, that was just missed during the removal, we can't link to libpcap any more. How do we want to take this patch in, and where is it needed? I hadn't planned to send a UML PR to -rc still, but I guess I _can_ if needed? But if anyone else wants to line it up through a tree (rust related?) that has pending work anyway, that seems fair too. In which case: Acked-by: Johannes Berg <johannes@sipsolutions.net> Or it's not that urgent because all this came up in -next now? I didn't really see (or fully understand) all the build bug reports. johannes ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arch: um: Don't rename vmap to kernel_vmap 2025-11-24 7:49 ` Johannes Berg @ 2025-11-25 7:36 ` David Gow 2025-11-25 7:40 ` Johannes Berg 0 siblings, 1 reply; 6+ messages in thread From: David Gow @ 2025-11-25 7:36 UTC (permalink / raw) To: Johannes Berg Cc: Miguel Ojeda, Richard Weinberger, Anton Ivanov, Miguel Ojeda, Paolo 'Blaisorblade' Giarrusso, Alice Ryhl, linux-um, linux-kernel, rust-for-linux [-- Attachment #1: Type: text/plain, Size: 2855 bytes --] On Mon, 24 Nov 2025 at 15:49, Johannes Berg <johannes@sipsolutions.net> wrote: > > On Sun, 2025-11-23 at 18:07 +0100, Miguel Ojeda wrote: > > 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") > > Indeed, that was just missed during the removal, we can't link to > libpcap any more. > > How do we want to take this patch in, and where is it needed? I hadn't > planned to send a UML PR to -rc still, but I guess I _can_ if needed? > But if anyone else wants to line it up through a tree (rust related?) > that has pending work anyway, that seems fair too. In which case: > > Acked-by: Johannes Berg <johannes@sipsolutions.net> > > Or it's not that urgent because all this came up in -next now? I didn't > really see (or fully understand) all the build bug reports. > I'm happy for this to go in via any tree. (Worst-case, we could possibly take it via KUnit, though I'd rather not, as it's not really KUnit-related at all.) The issue has actually been around since probably 6.16 (c284d3e42338 ("rust: drm: gem: Add GEM object abstraction")), but since it only applies to people building Rust graphics drivers against UML, which is not super common, it seems like it's only come up in randconfig builds so far. -- David [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 5281 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arch: um: Don't rename vmap to kernel_vmap 2025-11-25 7:36 ` David Gow @ 2025-11-25 7:40 ` Johannes Berg 2025-11-25 9:17 ` David Gow 0 siblings, 1 reply; 6+ messages in thread From: Johannes Berg @ 2025-11-25 7:40 UTC (permalink / raw) To: David Gow Cc: Miguel Ojeda, Richard Weinberger, Anton Ivanov, Miguel Ojeda, Paolo 'Blaisorblade' Giarrusso, Alice Ryhl, linux-um, linux-kernel, rust-for-linux On Tue, 2025-11-25 at 15:36 +0800, David Gow wrote: > > > > Or it's not that urgent because all this came up in -next now? I didn't > > really see (or fully understand) all the build bug reports. > > > > I'm happy for this to go in via any tree. (Worst-case, we could > possibly take it via KUnit, though I'd rather not, as it's not really > KUnit-related at all.) > > The issue has actually been around since probably 6.16 (c284d3e42338 > ("rust: drm: gem: Add GEM object abstraction")), but since it only > applies to people building Rust graphics drivers against UML, which is > not super common, it seems like it's only come up in randconfig builds > so far. Oh, interesting, OK. I guess then given that it's not super important and how late we're in the game, I'll just throw it into the (relatively small) pile we have for UML for -next. Given that we removed the pcap driver in 6.11 (12b8e7e69aa7a) I guess we could even ask stable to take it, but it's not even that important until someone wants to test the rust DRM stuff in kunit or something :) johannes ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arch: um: Don't rename vmap to kernel_vmap 2025-11-25 7:40 ` Johannes Berg @ 2025-11-25 9:17 ` David Gow 0 siblings, 0 replies; 6+ messages in thread From: David Gow @ 2025-11-25 9:17 UTC (permalink / raw) To: Johannes Berg Cc: Miguel Ojeda, Richard Weinberger, Anton Ivanov, Miguel Ojeda, Paolo 'Blaisorblade' Giarrusso, Alice Ryhl, linux-um, linux-kernel, rust-for-linux [-- Attachment #1: Type: text/plain, Size: 1438 bytes --] On Tue, 25 Nov 2025 at 15:40, Johannes Berg <johannes@sipsolutions.net> wrote: > > On Tue, 2025-11-25 at 15:36 +0800, David Gow wrote: > > > > > > Or it's not that urgent because all this came up in -next now? I didn't > > > really see (or fully understand) all the build bug reports. > > > > > > > I'm happy for this to go in via any tree. (Worst-case, we could > > possibly take it via KUnit, though I'd rather not, as it's not really > > KUnit-related at all.) > > > > The issue has actually been around since probably 6.16 (c284d3e42338 > > ("rust: drm: gem: Add GEM object abstraction")), but since it only > > applies to people building Rust graphics drivers against UML, which is > > not super common, it seems like it's only come up in randconfig builds > > so far. > > Oh, interesting, OK. I guess then given that it's not super important > and how late we're in the game, I'll just throw it into the (relatively > small) pile we have for UML for -next. Given that we removed the pcap > driver in 6.11 (12b8e7e69aa7a) I guess we could even ask stable to take > it, but it's not even that important until someone wants to test the > rust DRM stuff in kunit or something :) > Sounds good to me. Throwing the Fixes: tag in wouldn't hurt at least (and I do think some of the Rust DRM stuff is growing some KUnit tests, so it'll be nice to have going forward, even if they can be tested under other architectures). Cheers, -- David [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 5281 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-25 9:18 UTC | newest] Thread overview: 6+ 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 2025-11-24 7:49 ` Johannes Berg 2025-11-25 7:36 ` David Gow 2025-11-25 7:40 ` Johannes Berg 2025-11-25 9:17 ` David Gow
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).