All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Danilo Krummrich" <dakr@kernel.org>,
	"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>,
	"Mark Brown" <broonie@kernel.org>
Cc: "Alvin Sun" <alvin.sun@linux.dev>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Luis Chamberlain" <mcgrof@kernel.org>,
	"Petr Pavlu" <petr.pavlu@suse.com>,
	"Daniel Gomez" <da.gomez@kernel.org>,
	"Sami Tolvanen" <samitolvanen@google.com>,
	"Aaron Tomlin" <atomlin@atomlin.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	"David Gow" <david@davidgow.net>,
	"Rae Moar" <raemoar63@gmail.com>,
	"Breno Leitao" <leitao@debian.org>,
	"Jens Axboe" <axboe@kernel.dk>,
	"Dave Ertman" <david.m.ertman@intel.com>,
	"Leon Romanovsky" <leon@kernel.org>,
	"Igor Korotin" <igor.korotin@linux.dev>,
	"FUJITA Tomonori" <fujita.tomonori@gmail.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Todd Kjos" <tkjos@android.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Carlos Llamas" <cmllamas@google.com>,
	rust-for-linux@vger.kernel.org, linux-modules@vger.kernel.org,
	driver-core@lists.linux.dev, dri-devel@lists.freedesktop.org,
	nova-gpu@lists.linux.dev, linux-kselftest@vger.kernel.org,
	kunit-dev@googlegroups.com, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH v10 00/10] Fix missing fops.owner in Rust DRM/misc abstractions
Date: Thu, 13 Aug 2026 00:57:36 +0100	[thread overview]
Message-ID: <DKNDF3WD27GM.5VP6RBRXZBUS@garyguo.net> (raw)
In-Reply-To: <DKNAS52KYWLD.M15VEC6U0F6R@kernel.org>

On Wed Aug 12, 2026 at 10:53 PM BST, Danilo Krummrich wrote:
> (Cc: Mark)
>
> On Wed Aug 12, 2026 at 6:33 PM CEST, Miguel Ojeda wrote:
>> On Tue, Aug 11, 2026 at 8:41 AM Alvin Sun <alvin.sun@linux.dev> wrote:
>>>
>>> The series moves `THIS_MODULE` into the `ModuleMetadata` as a const, threads it
>>> through `#[vtable]` to set `fops.owner` in DRM/miscdevice, and updates configfs
>>> and rnull to use `this_module::<LocalModule>()`.
>>
>> Applied to `rust-next` -- thanks everyone!
>
> This series has a semantic conflict with both the driver-core and the drm-rust
> tree:
>
> @Mark: When you merge driver-core-next after rust-next (which I think is the
> case) then you need to include the diff in [1] into the merge.
>
> In drm-rust-next the build fails with:
>
> 	error[E0425]: cannot find type `LocalModule` in the crate root
> 	   --> rust/kernel/drm/gem/shmem.rs:628:5
> 	    |
> 	628 |     #[vtable]
> 	    |     ^^^^^^^^^ not found in the crate root
> 	    |
> 	    = note: this error originates in the attribute macro `vtable` (in Nightly builds, run with -Z macro-backtrace for more info)
>
> which is because the kunit test in rust/kernel/drm/gem/shmem.rs uses the
> #[vtable] macro.
>
> This should be fixed up with a patch on top of this series in rust-next.
>
> I came up with to potential solutions [2] and [3]. I think with the new build
> system we want [3], but I'm not entirely sure this works correctly with the
> current build system in all cases (at least it did survive my tests).
>
> Alternatively, we could just open-code a dummy module as in [2] for now.
>
> [1] driver-core-next merge fixup
>
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index a4927452016e..17ca504b7f8d 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
> @@ -87,7 +87,7 @@ unsafe fn register(
>          }
>
>          // SAFETY: `sdrv` is guaranteed to be a valid `DriverType`.
> -        to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) })
> +        to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.as_ptr()) })

`module.0` shouldn't be used in the first place, it just happens to be visible
due to the unfortunate placement at crate root.

Perhaps you can update driver-core tree to use `as_ptr()`? It was already there
and not newly introduced in the series.

>      }
>
>      unsafe fn unregister(sdrv: &Opaque<Self::DriverType>) {
>
> [2] Open-coded dummy module
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index c04e6c5aa7e0..274924cfcc05 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -157,6 +157,15 @@
>  /// Prefix to appear before log messages printed from within the `kernel` crate.
>  const __LOG_PREFIX: &[u8] = b"rust_kernel\0";
>
> +/// Dummy module type for `#[vtable]` impl blocks within the kernel crate (e.g. kunit tests).
> +struct LocalModule;
> +
> +impl ModuleMetadata for LocalModule {
> +    const NAME: &'static str::CStr = c"rust_kernel";
> +    // SAFETY: `try_module_get`/`module_put` handle null module pointers gracefully.
> +    const THIS_MODULE: ThisModule = unsafe { ThisModule::from_ptr(core::ptr::null_mut()) };
> +}
> +
>  #[cfg(not(testlib))]
>  #[panic_handler]
>  fn panic(info: &core::panic::PanicInfo<'_>) -> ! {

IMO this is the correct way, also consistent with

https://lore.kernel.org/rust-for-linux/20260811-fix-fops-owner-v10-3-7e71776f9dbe@linux.dev/

It might make sense to add this to rust-next.

Best,
Gary

  reply	other threads:[~2026-08-12 23:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11  6:39 [PATCH v10 00/10] Fix missing fops.owner in Rust DRM/misc abstractions Alvin Sun
2026-08-11  6:39 ` [PATCH v10 01/10] rust: module: move module types into `module.rs` Alvin Sun
2026-08-11  6:39 ` [PATCH v10 02/10] rust: module: add `THIS_MODULE` const to `ModuleMetadata` trait Alvin Sun
2026-08-11  6:39 ` [PATCH v10 03/10] rust: doctest: add LocalModule fallback for #[vtable] ThisModule Alvin Sun
2026-08-11  6:39 ` [PATCH v10 04/10] rust: macros: auto-insert OwnerModule in #[vtable] Alvin Sun
2026-08-11  6:39 ` [PATCH v10 05/10] rust: drm: set fops.owner from driver module pointer Alvin Sun
2026-08-11  6:39 ` [PATCH v10 06/10] rust: miscdevice: " Alvin Sun
2026-08-11  6:39 ` [PATCH v10 07/10] rust: configfs: use `LocalModule` for `THIS_MODULE` Alvin Sun
2026-08-11  6:39 ` [PATCH v10 08/10] rust_binder: " Alvin Sun
2026-08-11  6:39 ` [PATCH v10 09/10] rust: macros: remove `THIS_MODULE` static from `module!` Alvin Sun
2026-08-11  6:39 ` [PATCH v10 10/10] rust: module: update MAINTAINERS to cover module.rs Alvin Sun
2026-08-12 16:33 ` [PATCH v10 00/10] Fix missing fops.owner in Rust DRM/misc abstractions Miguel Ojeda
2026-08-12 21:53   ` Danilo Krummrich
2026-08-12 23:57     ` Gary Guo [this message]
2026-08-13  0:18       ` Danilo Krummrich

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=DKNDF3WD27GM.5VP6RBRXZBUS@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=alvin.sun@linux.dev \
    --cc=arnd@arndb.de \
    --cc=arve@android.com \
    --cc=atomlin@atomlin.com \
    --cc=axboe@kernel.dk \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=brauner@kernel.org \
    --cc=brendan.higgins@linux.dev \
    --cc=broonie@kernel.org \
    --cc=cmllamas@google.com \
    --cc=da.gomez@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=david.m.ertman@intel.com \
    --cc=david@davidgow.net \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=driver-core@lists.linux.dev \
    --cc=fujita.tomonori@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=igor.korotin@linux.dev \
    --cc=kunit-dev@googlegroups.com \
    --cc=kwilczynski@kernel.org \
    --cc=leitao@debian.org \
    --cc=leon@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=raemoar63@gmail.com \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=samitolvanen@google.com \
    --cc=simona@ffwll.ch \
    --cc=tkjos@android.com \
    --cc=tmgross@umich.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.