rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Andrew Lunn <andrew@lunn.ch>, Alice Ryhl <aliceryhl@google.com>,
	fujita.tomonori@gmail.com, benno.lossin@proton.me,
	miguel.ojeda.sandonis@gmail.com, netdev@vger.kernel.org,
	rust-for-linux@vger.kernel.org, tmgross@umich.edu,
	wedsonaf@gmail.com
Subject: Re: [PATCH net-next v7 1/5] rust: core abstractions for network PHY drivers
Date: Fri, 17 Nov 2023 15:28:46 -0800	[thread overview]
Message-ID: <ZVf3LvoZ7npy3WxI@boqun-archlinux> (raw)
In-Reply-To: <2023111709-amiable-everybody-befb@gregkh>

On Fri, Nov 17, 2023 at 02:50:45PM -0500, Greg KH wrote:
> On Fri, Nov 17, 2023 at 02:53:44PM +0100, Andrew Lunn wrote:
> > > I would change this to "it's okay to call phy_drivers_unregister from a
> > > different thread than the one in which phy_drivers_register was called".
> > 
> > This got me thinking about 'threads'. For register and unregister, we

Just to make things clear for discussion, the "thread" here (when we are
talking about trait `Send` and `Sync`) means "contexts" (thread/process
contexts, irq contexts, etc).

When we say a type is `Send`, it means the object of that type can be
created in one context, passed to another context (could be the same
type of context, e.g. sending between two thread/process contexts) and
dropped there. For example, if you have a work_struct embedded type, you
can create it in the driver code and pass it to workqueue, and when the
work is done, you can free it in the workqueue context.

One example of not `Send` type (or `!Send`) is spinlock guard:

	let guard: Guard<..> = some_lock.lock();

creating a Guard means "spin_lock()" and dropping a Guard means
"spin_unlock()", since we cannot acquire a spinlock in one context and
release it in another context in kernel, so `Guard<..>` is `!Send`.

Back to the code here:

	unsafe impl Send for Registration {}

the safety comment needs to explain why the `Registration::drop` is safe
to call in a different context.

Hope this helps.

Regards,
Boqun

> > are talking abut the kernel modules module_init() and module_exit()
> > function. module_init() can be called before user space is even
> > started, but it could also be called by insmod. module_exit() could be
> > called by rmmod, but it could also be the kernel, after user space has
> > gone away on shutdown.
> 
> The kernel will not call module_exit() on shutdown.  Or has something
> changed recently?
> 
> > We are always in a context which can block, but
> > i never really think of this being threads. You can start a kernel
> > thread, and have some data structure exclusively used by that kernel
> > thread, but that is pretty unusual.
> > 
> > So i would probably turn this commenting around. Only comment like
> > this in the special case that a kernel thread exists, and it is
> > expected to have exclusive access.
> 
> With the driver model, you can be sure that your probe/release functions
> for the bus will never be called at the same time, and module_init/exit
> can never be called at the same time, so perhaps this isn't an issue?
> 
> thanks,
> 
> greg k-h
> 

  reply	other threads:[~2023-11-17 23:28 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-26  0:10 [PATCH net-next v7 0/5] Rust abstractions for network PHY drivers FUJITA Tomonori
2023-10-26  0:10 ` [PATCH net-next v7 1/5] rust: core " FUJITA Tomonori
2023-10-27 19:09   ` Boqun Feng
2023-10-28 10:00     ` FUJITA Tomonori
2023-10-27 19:59   ` Boqun Feng
2023-10-27 21:19     ` Benno Lossin
2023-10-27 22:21       ` Boqun Feng
2023-10-27 22:36         ` Andrew Lunn
2023-10-27 22:50         ` Benno Lossin
2023-10-27 23:26           ` Boqun Feng
2023-10-27 23:52             ` Boqun Feng
2023-10-28  8:35             ` Benno Lossin
2023-10-27 22:40       ` Andrew Lunn
2023-10-28 15:16         ` Miguel Ojeda
2023-10-28 18:18           ` Andrew Lunn
2023-10-28  9:27       ` FUJITA Tomonori
2023-10-28 14:53         ` Andrew Lunn
2023-10-28 16:09           ` FUJITA Tomonori
2023-10-28 16:39             ` Benno Lossin
2023-10-28 19:06               ` Boqun Feng
2023-10-28 19:23                 ` Andrew Lunn
2023-10-28 23:26                   ` Boqun Feng
2023-10-28 16:37         ` Benno Lossin
2023-10-28 18:23           ` Andrew Lunn
2023-10-28 18:45             ` Benno Lossin
2023-10-29  4:21               ` FUJITA Tomonori
2023-10-29 16:48                 ` Boqun Feng
2023-10-29 18:09                   ` Boqun Feng
2023-10-29 18:26                     ` Boqun Feng
2023-10-29 19:39                     ` Andrew Lunn
2023-10-30 12:07                       ` Miguel Ojeda
2023-10-30 12:32                         ` Andrew Lunn
2023-10-29 22:58                   ` FUJITA Tomonori
2023-10-30  0:19                     ` Boqun Feng
2023-10-30  8:34                       ` Benno Lossin
2023-10-30 12:49                         ` FUJITA Tomonori
2023-10-30 16:45                           ` Benno Lossin
2023-11-08 10:46                             ` FUJITA Tomonori
2023-11-10 13:26                               ` Andrew Lunn
2023-10-29 17:32                 ` Andrew Lunn
2023-10-30  8:37                   ` Benno Lossin
2023-10-30 11:22             ` Miguel Ojeda
2023-11-17  9:39   ` Alice Ryhl
2023-11-17 13:34     ` Andrew Lunn
2023-11-17 15:42       ` Alice Ryhl
2023-11-17 16:28         ` Andrew Lunn
2023-11-17 18:27           ` Alice Ryhl
2023-11-21 12:47         ` FUJITA Tomonori
2023-11-17  9:39   ` Alice Ryhl
2023-11-17 13:53     ` Andrew Lunn
2023-11-17 19:50       ` Greg KH
2023-11-17 23:28         ` Boqun Feng [this message]
2023-11-18 15:32           ` Andrew Lunn
2023-11-18 15:54             ` Boqun Feng
2023-11-19 11:06               ` Trevor Gross
2023-11-21  2:13                 ` FUJITA Tomonori
2023-11-22 18:16                   ` Boqun Feng
2023-11-19 13:51     ` FUJITA Tomonori
2023-11-19 16:08       ` Andrew Lunn
2023-10-26  0:10 ` [PATCH net-next v7 2/5] rust: net::phy add module_phy_driver macro FUJITA Tomonori
2023-11-17  9:39   ` Alice Ryhl
2023-11-19 10:50     ` FUJITA Tomonori
2023-11-19 10:54       ` Benno Lossin
2023-11-17 22:21   ` Boqun Feng
2023-11-17 22:54     ` Andrew Lunn
2023-11-17 23:01       ` Benno Lossin
2023-11-17 23:18         ` Andrew Lunn
2023-11-19  9:41           ` FUJITA Tomonori
2023-11-19  9:25         ` FUJITA Tomonori
2023-11-19 15:50           ` Andrew Lunn
2023-11-20 13:54             ` FUJITA Tomonori
2023-11-20 14:13               ` Andrew Lunn
2023-11-21  0:49                 ` FUJITA Tomonori
2023-11-19  9:44     ` FUJITA Tomonori
2023-10-26  0:10 ` [PATCH net-next v7 3/5] rust: add second `bindgen` pass for enum exhaustiveness checking FUJITA Tomonori
2023-10-26 11:02   ` Miguel Ojeda
2023-10-26 11:54     ` FUJITA Tomonori
2023-10-26 12:22       ` Miguel Ojeda
2023-10-27  0:07         ` Andrew Lunn
2023-10-27 10:50           ` Miguel Ojeda
2023-10-26  0:10 ` [PATCH net-next v7 4/5] MAINTAINERS: add Rust PHY abstractions for ETHERNET PHY LIBRARY FUJITA Tomonori
2023-10-26 23:53   ` Andrew Lunn
2023-10-26  0:10 ` [PATCH net-next v7 5/5] net: phy: add Rust Asix PHY driver FUJITA Tomonori
2023-11-17  9:39   ` Alice Ryhl
2023-11-19  9:57     ` FUJITA Tomonori
2023-11-19 16:03       ` Andrew Lunn
2023-11-21  6:19         ` FUJITA Tomonori
2023-11-21  7:12           ` Greg KH
2023-10-26 10:39 ` [PATCH net-next v7 0/5] Rust abstractions for network PHY drivers Miguel Ojeda
2023-10-26 23:48   ` Andrew Lunn
2023-10-27  2:06     ` Boqun Feng
2023-10-27  2:47       ` Andrew Lunn
2023-10-27  3:11         ` Boqun Feng
2023-10-27  4:26           ` Boqun Feng
2023-10-27 14:26             ` Andrew Lunn
2023-10-27 16:41               ` Miguel Ojeda
2023-10-27 13:00           ` Andrew Lunn
2023-10-27 10:22         ` Miguel Ojeda
2023-10-27 13:09           ` Andrew Lunn
2023-10-27 10:21     ` Miguel Ojeda
2023-10-27 14:26       ` Jakub Kicinski
2023-10-27 16:36         ` Miguel Ojeda
2023-10-27 22:55           ` Andrew Lunn
2023-10-28 11:07             ` Miguel Ojeda
2023-10-28 11:41               ` Benno Lossin
2023-10-28 15:11                 ` Miguel Ojeda
2023-10-28 15:00               ` Andrew Lunn
2023-10-28 15:11                 ` Miguel Ojeda

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=ZVf3LvoZ7npy3WxI@boqun-archlinux \
    --to=boqun.feng@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=andrew@lunn.ch \
    --cc=benno.lossin@proton.me \
    --cc=fujita.tomonori@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=wedsonaf@gmail.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 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).