rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Gow <davidgow@google.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	 Anton Ivanov <anton.ivanov@cambridgegreys.com>,
	Miguel Ojeda <ojeda@kernel.org>,
	 "Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it>,
	Alice Ryhl <aliceryhl@google.com>,
	 linux-um@lists.infradead.org, linux-kernel@vger.kernel.org,
	 rust-for-linux@vger.kernel.org
Subject: Re: [PATCH] arch: um: Don't rename vmap to kernel_vmap
Date: Tue, 25 Nov 2025 15:36:31 +0800	[thread overview]
Message-ID: <CABVgOSmAGahEaa71_fOT4Oo9Wg91X6rN+1_KgVVvVzmd4vuMCg@mail.gmail.com> (raw)
In-Reply-To: <bfb512e24f365695534313b375fc0c38d0a843b2.camel@sipsolutions.net>

[-- 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 --]

  reply	other threads:[~2025-11-25  7:36 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2025-11-25  7:40       ` Johannes Berg
2025-11-25  9:17         ` David Gow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABVgOSmAGahEaa71_fOT4Oo9Wg91X6rN+1_KgVVvVzmd4vuMCg@mail.gmail.com \
    --to=davidgow@google.com \
    --cc=aliceryhl@google.com \
    --cc=anton.ivanov@cambridgegreys.com \
    --cc=blaisorblade@yahoo.it \
    --cc=johannes@sipsolutions.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=richard@nod.at \
    --cc=rust-for-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).