rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benno Lossin <benno.lossin@proton.me>
To: FUJITA Tomonori <fujita.tomonori@gmail.com>
Cc: netdev@vger.kernel.org, rust-for-linux@vger.kernel.org,
	andrew@lunn.ch, miguel.ojeda.sandonis@gmail.com,
	tmgross@umich.edu, boqun.feng@gmail.com, wedsonaf@gmail.com,
	greg@kroah.com
Subject: Re: [PATCH net-next v5 1/5] rust: core abstractions for network PHY drivers
Date: Sat, 21 Oct 2023 13:05:59 +0000	[thread overview]
Message-ID: <fb45d4aa-2816-4457-93e9-aec72f8ec64e@proton.me> (raw)
In-Reply-To: <20231021.220012.2089903288409349337.fujita.tomonori@gmail.com>

On 21.10.23 15:00, FUJITA Tomonori wrote:
> On Sat, 21 Oct 2023 12:50:10 +0000
> Benno Lossin <benno.lossin@proton.me> wrote:
>>>>>> I think this is very weird, do you have any idea why this
>>>>>> could happen?
>>>>>
>>>>> DriverVtable is created on kernel stack, I guess.
>>>>
>>>> But how does that invalidate the function pointers?
>>>
>>> Not only funciton pointers. You can't store something on stack for
>>> later use.
>>
>> It is not stored on the stack, it is only created on the stack and
>> moved to a global static later on. The `module!` macro creates a
>> `static mut __MOD: Option<Module>` where the module data is stored in.
> 
> I know. The problem is that we call phy_drivers_register() with
> DriverVTable on stack. Then it was moved.

I see, what exactly is the problem with that? In other words:
why does PHYLIB need `phy_driver` to stay at the same address?

This is an important requirement in Rust. Rust can ensure that
types are not moved by means of pinning them. In this case, Wedson's
patch below should fix the issue completely.

But we should also fix this in the abstractions, the `DriverVTable`
type should only be constructible in a pinned state. For this purpose
we have the `pin-init` API [2].

Are there any other things in PHY that must not change address?

[2]: https://rust-for-linux.github.io/docs/v6.6-rc2/kernel/init/index.html

-- 
Cheers,
Benno

>> It seems that constructing the driver table not at that location
>> is somehow interfering with something?
>>
>> Wedson has a patch [1] to create in-place initialized modules, but
>> it probably is not completely finished, as he has not yet begun to
>> post it to the list. But I am sure that it is mature enough for
>> you to test this hypothesis.
>>
>> [1]: https://github.com/wedsonaf/linux/commit/484ec70025ff9887d9ca228ec631264039cee35


  reply	other threads:[~2023-10-21 13:06 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-17 11:30 [PATCH net-next v5 0/5] Rust abstractions for network PHY drivers FUJITA Tomonori
2023-10-17 11:30 ` [PATCH net-next v5 1/5] rust: core " FUJITA Tomonori
2023-10-18 15:07   ` Benno Lossin
2023-10-19  0:24     ` FUJITA Tomonori
2023-10-19 13:45       ` Benno Lossin
2023-10-19 14:42         ` FUJITA Tomonori
2023-10-19 15:20           ` Benno Lossin
2023-10-19 15:32             ` FUJITA Tomonori
2023-10-19 16:37               ` Benno Lossin
2023-10-19 21:51                 ` FUJITA Tomonori
2023-10-21  7:21                   ` Benno Lossin
2023-10-20  0:34             ` FUJITA Tomonori
2023-10-20 12:54               ` FUJITA Tomonori
2023-10-21  7:25                 ` Benno Lossin
2023-10-21  7:30                   ` FUJITA Tomonori
2023-10-21  8:37                     ` Benno Lossin
2023-10-21 10:27                       ` FUJITA Tomonori
2023-10-21 11:21                         ` Benno Lossin
2023-10-21 11:36                           ` FUJITA Tomonori
2023-10-21 12:13                             ` Benno Lossin
2023-10-21 12:38                               ` FUJITA Tomonori
2023-10-21 12:50                                 ` Benno Lossin
2023-10-21 13:00                                   ` FUJITA Tomonori
2023-10-21 13:05                                     ` Benno Lossin [this message]
2023-10-21 13:31                                       ` FUJITA Tomonori
2023-10-21 13:35                                         ` Benno Lossin
2023-10-21 21:45                                           ` FUJITA Tomonori
2023-10-23  6:35                                             ` Benno Lossin
2023-10-23  6:37                                               ` Benno Lossin
2023-10-21 15:57                                       ` Andrew Lunn
2023-10-21 16:31                                         ` Benno Lossin
2023-10-21 15:41                             ` Andrew Lunn
2023-10-20 18:42     ` Andrew Lunn
2023-10-21  4:44       ` FUJITA Tomonori
2023-10-21  7:36       ` Benno Lossin
2023-10-21 12:47       ` Miguel Ojeda
2023-10-22  9:47         ` FUJITA Tomonori
2023-10-22 11:37           ` Miguel Ojeda
2023-10-22 15:34             ` Andrew Lunn
2023-10-24  1:37               ` FUJITA Tomonori
2023-10-24  8:48               ` Miguel Ojeda
2023-10-18 20:27   ` Andrew Lunn
2023-10-19  0:41     ` FUJITA Tomonori
2023-10-19 13:57       ` Benno Lossin
2023-10-20 19:50         ` Andrew Lunn
2023-10-21  8:01           ` Benno Lossin
2023-10-21 15:35             ` Andrew Lunn
2023-10-20 17:26   ` Andreas Hindborg (Samsung)
2023-10-20 17:56     ` Benno Lossin
2023-10-20 19:59     ` Andrew Lunn
2023-10-20 20:30       ` Andreas Hindborg (Samsung)
2023-10-21  3:49         ` FUJITA Tomonori
2023-10-21  4:01     ` FUJITA Tomonori
2023-10-17 11:30 ` [PATCH net-next v5 2/5] rust: net::phy add module_phy_driver macro FUJITA Tomonori
2023-10-17 11:30 ` [PATCH net-next v5 3/5] WIP rust: add second `bindgen` pass for enum exhaustiveness checking FUJITA Tomonori
2023-10-20 11:37   ` Andreas Hindborg (Samsung)
2023-10-20 12:34     ` Miguel Ojeda
2023-10-20 12:35       ` Miguel Ojeda
2023-10-23  8:57       ` Andreas Hindborg (Samsung)
2023-10-21  3:51     ` FUJITA Tomonori
2023-10-21 12:05       ` Miguel Ojeda
2023-10-22  6:30         ` FUJITA Tomonori
2023-10-23  8:58         ` Andreas Hindborg (Samsung)
2023-10-17 11:30 ` [PATCH net-next v5 4/5] MAINTAINERS: add Rust PHY abstractions for ETHERNET PHY LIBRARY FUJITA Tomonori
2023-10-17 11:30 ` [PATCH net-next v5 5/5] net: phy: add Rust Asix PHY driver FUJITA Tomonori

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=fb45d4aa-2816-4457-93e9-aec72f8ec64e@proton.me \
    --to=benno.lossin@proton.me \
    --cc=andrew@lunn.ch \
    --cc=boqun.feng@gmail.com \
    --cc=fujita.tomonori@gmail.com \
    --cc=greg@kroah.com \
    --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).