[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250320013913.1624829-1-contact@antoniohickey.com>
Date: Wed, 19 Mar 2025 21:39:13 -0400
From: Antonio Hickey <contact@...oniohickey.com>
To: davidgow@...gle.com
Cc: a.hindborg@...nel.org,
alex.gaynor@...il.com,
aliceryhl@...gle.com,
benno.lossin@...ton.me,
bjorn3_gh@...tonmail.com,
boqun.feng@...il.com,
brendan.higgins@...ux.dev,
contact@...oniohickey.com,
contact@...e-forge.io,
dakr@...nel.org,
gary@...yguo.net,
kunit-dev@...glegroups.com,
linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org,
ojeda@...nel.org,
rmoar@...gle.com,
rust-for-linux@...r.kernel.org,
tmgross@...ch.edu
Subject: Re: [PATCH v4 08/16] rust: kunit: refactor to use `&raw [const|mut]`
On Tue, Mar 18, 2025 at 04:02:15PM +0800, David Gow wrote:
> On Sun, 16 Mar 2025 at 14:20, Antonio Hickey <contact@...e-forge.io> wrote:
> >
> > Replacing all occurrences of `addr_of!(place)` and `addr_of_mut!(place)`
> > with `&raw const place` and `&raw mut place` respectively.
> >
> > This will allow us to reduce macro complexity, and improve consistency
> > with existing reference syntax as `&raw const`, `&raw mut` are similar
> > to `&`, `&mut` making it fit more naturally with other existing code.
> >
> > Suggested-by: Benno Lossin <benno.lossin@...ton.me>
> > Link: https://github.com/Rust-for-Linux/linux/issues/1148
> > Signed-off-by: Antonio Hickey <contact@...oniohickey.com>
> > ---
>
> Thanks, Antonio.
>
> So this looks fine, but it's also a bit annoying, as the remaining
> KUnit/Rust integration patches[1] were recently updated to use
> `addr_of_mut!()`, so either this patch, or those, will need updating.
>
> In general, I think changes such as those in this series are going to
> get progressively more prone to conflicts as Rust is adopted by other
> subsystems, as the 'rust' tree won't be the only one carrying changes
> which could be affected. Maybe in the future it'd make sense to split
> these up into a series which enables the new feature, and only then
> put the warnings in place in the next version?
>
Hi David,
I understand your general frustration around this, but I do
think it's better to have a higher frequency of change now while
Rust adoption is still early than later on. I know you're looking
at this in a more forward lense of the future, and I think you have
a fair point that this can be more problematic as adoption increases.
> In the KUnit case in particular, since the patches haven't actually
> been merged yet, we have three options:
> - Merge this into rust-next, and send out a new version of the KUnit
> patches which depend on it, which then are also carried in rust-next,
> or delayed (again) to 6.16. I suspect given how close to the merge
> window we are, the delay is more likely.
> - Merge the KUnit changes as-is (either into rust-next or
> kselftest/kunit), and send out a new version of this series which also
> updates it (probably in 6.16, but maybe sooner if everything goes via
> rust-next).
> - Merge both, and accept that there'll be some clippy errors until a
> follow-up patch fixing them is sent and merged.
I'm currently preparing to send out a new version of my patch right
now. Depending on how the timing of our patches plays out, I'm not
opposed to updating your patches KUnit changes within my patch set, as
it's very much aligned with the scope of my patch set.
>
> As someone with a vested interest in the KUnit changes, and at best a
> mild academic interest in the addr_of_muit!() deprecation, my
> preferences are with the latter two options. (I'd also, in general,
> whinge that the frequency of new Rust versions / warnings is high
> enough that it's taking a not insignificant portion of the limited
> time I have working on Rust things to understand and deal with the
> churn.)
>
> Regardless, apart from the minor irritation of having to learn yet
> another new syntax, I have no objections to this patch in particular.
>
> Reviewed-by: David Gow <davidgow@...gle.com>
>
> Thanks (and sorry for the grumpiness: don't take it personally),
> -- David
Haha no problem, you have a fair point.
Thanks,
Antonio
>
> [1]: https://lore.kernel.org/rust-for-linux/20250307090103.918788-1-davidgow@google.com/
Powered by blists - more mailing lists