[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c3fbe144-9b17-4e25-929d-9785901dfaa4@proton.me>
Date: Mon, 30 Oct 2023 08:37:33 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Andrew Lunn <andrew@...n.ch>, FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: boqun.feng@...il.com, netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, tmgross@...ch.edu, miguel.ojeda.sandonis@...il.com, wedsonaf@...il.com
Subject: Re: [PATCH net-next v7 1/5] rust: core abstractions for network PHY drivers
On 29.10.23 18:32, Andrew Lunn wrote:
>> The current code is fine from Rust perspective because the current
>> code copies phy_driver on stack and makes a reference to the copy, if
>> I undertand correctly.
>>
>> It's not nice to create an 500-bytes object on stack. It turned out
>> that it's not so simple to avoid it.
>
> Does it also copy the stack version over the 'real' version before
> exiting? If not, it is broken, since modifying state in phy_device is
> often why the driver is called. But copying the stack version is also
> broken, since another thread taking the phydev->lock is going to get
> lost from the linked list of waiters.
It does not copy the stack version over the original. Since we only read
the `link` field in the discussed function and we hold the lock, it should
not get modified, right? So from a functional viewpoint there is no issue.
(Aside from increased stack size and the data race issue etc.)
> Taking a copy of the C structure does seem very odd, to me as a C
> programmer.
It is also odd in Rust.
--
Cheers,
Benno
Powered by blists - more mailing lists