All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Danilo Krummrich <dakr@kernel.org>
Cc: rafael@kernel.org, igor.korotin.linux@gmail.com,
	ojeda@kernel.org, boqun.feng@gmail.com, gary@garyguo.net,
	bjorn3_gh@protonmail.com, lossin@kernel.org,
	a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu,
	david.m.ertman@intel.com, ira.weiny@intel.com, leon@kernel.org,
	bhelgaas@google.com, kwilczynski@kernel.org,
	wsa+renesas@sang-engineering.com, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-usb@vger.kernel.org, linux-i2c@vger.kernel.org
Subject: Re: [PATCH 6/6] rust: driver: drop device private data post unbind
Date: Wed, 7 Jan 2026 13:22:44 +0100	[thread overview]
Message-ID: <2026010741-wiry-trophy-46ec@gregkh> (raw)
In-Reply-To: <20260107103511.570525-7-dakr@kernel.org>

On Wed, Jan 07, 2026 at 11:35:05AM +0100, Danilo Krummrich wrote:
> @@ -548,6 +548,10 @@ static DEVICE_ATTR_RW(state_synced);
>  static void device_unbind_cleanup(struct device *dev)
>  {
>  	devres_release_all(dev);
> +#ifdef CONFIG_RUST

Nit, let's not put #ifdef in .c files, the overhead of an empty pointer
for all drivers is not a big deal.

> +	if (dev->driver->p_cb.post_unbind)
> +		dev->driver->p_cb.post_unbind(dev);
> +#endif
>  	arch_teardown_dma_ops(dev);
>  	kfree(dev->dma_range_map);
>  	dev->dma_range_map = NULL;
> diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
> index cd8e0f0a634b..51a9ebdd8a2d 100644
> --- a/include/linux/device/driver.h
> +++ b/include/linux/device/driver.h
> @@ -85,6 +85,8 @@ enum probe_type {
>   *		uevent.
>   * @p:		Driver core's private data, no one other than the driver
>   *		core can touch this.
> + * @p_cb:	Callbacks private to the driver core; no one other than the
> + *		driver core is allowed to touch this.
>   *
>   * The device driver-model tracks all of the drivers known to the system.
>   * The main reason for this tracking is to enable the driver core to match
> @@ -119,6 +121,15 @@ struct device_driver {
>  	void (*coredump) (struct device *dev);
>  
>  	struct driver_private *p;
> +#ifdef CONFIG_RUST

Again, no #ifdef.

> +	struct {
> +		/*
> +		 * Called after remove() and after all devres entries have been
> +		 * processed.
> +		 */
> +		void (*post_unbind)(struct device *dev);

post_unbind_rust_only()?

> -impl<T: RegistrationOps> Registration<T> {
> +impl<T: RegistrationOps + 'static> Registration<T> {
> +    extern "C" fn post_unbind_callback(dev: *mut bindings::device) {
> +        // SAFETY: The driver core only ever calls the post unbind callback with a valid pointer to
> +        // a `struct device`.
> +        //
> +        // INVARIANT: `dev` is valid for the duration of the `post_unbind_callback()`.
> +        let dev = unsafe { &*dev.cast::<device::Device<device::CoreInternal>>() };
> +
> +        // `remove()` and all devres callbacks have been completed at this point, hence drop the
> +        // driver's device private data.
> +        //
> +        // SAFETY: By the safety requirements of the `Driver` trait, `T::DriverData` is the
> +        // driver's device private data.
> +        drop(unsafe { dev.drvdata_obtain::<T::DriverData>() });

I don't mind this, but why don't we also do this for all C drivers?
Just null out the pointer at this point in time so that no one can touch
it, just like you are doing here (in a way.)

thanks,

greg k-h

  reply	other threads:[~2026-01-07 12:22 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-07 10:34 [PATCH 0/6] Address race condition with Device::drvdata() Danilo Krummrich
2026-01-07 10:35 ` [PATCH 1/6] rust: i2c: do not drop device private data on shutdown() Danilo Krummrich
2026-01-07 10:35 ` [PATCH 2/6] rust: auxiliary: add Driver::unbind() callback Danilo Krummrich
2026-01-07 10:35 ` [PATCH 3/6] rust: driver: introduce a common Driver trait Danilo Krummrich
2026-01-14 19:40   ` Igor Korotin
2026-01-07 10:35 ` [PATCH 4/6] rust: driver: add DEVICE_DRIVER_OFFSET to the " Danilo Krummrich
2026-01-07 10:35 ` [PATCH 5/6] rust: driver: add DriverData type to the generic " Danilo Krummrich
2026-01-07 10:35 ` [PATCH 6/6] rust: driver: drop device private data post unbind Danilo Krummrich
2026-01-07 12:22   ` Greg KH [this message]
2026-01-07 12:50     ` Danilo Krummrich
2026-01-07 14:54       ` Greg KH
2026-01-12 14:27         ` Danilo Krummrich
2026-01-12 15:03           ` Greg KH
2026-01-07 15:51 ` [PATCH 0/6] Address race condition with Device::drvdata() Alice Ryhl
2026-01-07 16:40   ` Danilo Krummrich
2026-01-12 15:34     ` Alice Ryhl
2026-01-12 15:47       ` Danilo Krummrich
2026-01-14 19:50 ` Igor Korotin
2026-01-16  0:23 ` 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=2026010741-wiry-trophy-46ec@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=david.m.ertman@intel.com \
    --cc=gary@garyguo.net \
    --cc=igor.korotin.linux@gmail.com \
    --cc=ira.weiny@intel.com \
    --cc=kwilczynski@kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=wsa+renesas@sang-engineering.com \
    /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.