| Message ID | db0166341ce824c157d0c58c240b3efc6aec6f6e.1738832118.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | Rust bindings for cpufreq and OPP core + sample driver | expand |
On Thu, Feb 06, 2025 at 02:58:25PM +0530, Viresh Kumar wrote: > In order to prepare for adding Rust abstractions for cpumask, this patch > adds cpumask helpers. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > MAINTAINERS | 1 + > rust/bindings/bindings_helper.h | 1 + > rust/helpers/cpumask.c | 40 +++++++++++++++++++++++++++++++++ > rust/helpers/helpers.c | 1 + > 4 files changed, 43 insertions(+) > create mode 100644 rust/helpers/cpumask.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ee6709599df5..bfc1bf2ebd77 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4021,6 +4021,7 @@ F: lib/cpumask_kunit.c > F: lib/find_bit.c > F: lib/find_bit_benchmark.c > F: lib/test_bitmap.c > +F: rust/helpers/cpumask.c Sorry what? I never committed to maintain this thing, and will definitely not do it for free. NAK. > F: tools/include/linux/bitfield.h > F: tools/include/linux/bitmap.h > F: tools/include/linux/bits.h > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > index fda1e0d8f376..59b4bc49d039 100644 > --- a/rust/bindings/bindings_helper.h > +++ b/rust/bindings/bindings_helper.h > @@ -11,6 +11,7 @@ > #include <linux/blk_types.h> > #include <linux/blkdev.h> > #include <linux/cpu.h> > +#include <linux/cpumask.h> > #include <linux/cred.h> > #include <linux/errname.h> > #include <linux/ethtool.h> > diff --git a/rust/helpers/cpumask.c b/rust/helpers/cpumask.c > new file mode 100644 > index 000000000000..49267ad33b3c > --- /dev/null > +++ b/rust/helpers/cpumask.c > @@ -0,0 +1,40 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/cpumask.h> > + > +void rust_helper_cpumask_set_cpu(unsigned int cpu, struct cpumask *dstp) > +{ > + cpumask_set_cpu(cpu, dstp); > +} > + > +void rust_helper_cpumask_clear_cpu(int cpu, struct cpumask *dstp) > +{ > + cpumask_clear_cpu(cpu, dstp); > +} > + > +void rust_helper_cpumask_setall(struct cpumask *dstp) > +{ > + cpumask_setall(dstp); > +} > + > +unsigned int rust_helper_cpumask_weight(struct cpumask *srcp) > +{ > + return cpumask_weight(srcp); > +} > + > +void rust_helper_cpumask_copy(struct cpumask *dstp, const struct cpumask *srcp) > +{ > + cpumask_copy(dstp, srcp); > +} > + > +bool rust_helper_zalloc_cpumask_var(cpumask_var_t *mask, gfp_t flags) > +{ > + return zalloc_cpumask_var(mask, flags); > +} > + > +#ifndef CONFIG_CPUMASK_OFFSTACK > +void rust_helper_free_cpumask_var(cpumask_var_t mask) > +{ > + free_cpumask_var(mask); > +} > +#endif This is most likely wrong because free_cpumask_var() is declared unconditionally, and I suspect the rust helper should be as well. > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > index 0640b7e115be..de2341cfd917 100644 > --- a/rust/helpers/helpers.c > +++ b/rust/helpers/helpers.c > @@ -11,6 +11,7 @@ > #include "bug.c" > #include "build_assert.c" > #include "build_bug.c" > +#include "cpumask.c" > #include "cred.c" > #include "device.c" > #include "err.c" > -- > 2.31.1.272.g89b43f80a514 Please for the next iteration keep me CCed for the whole series. I want to make sure you'll not make me a rust maintainer accidentally. Thanks, Yury
On 10-02-25, 19:02, Yury Norov wrote: > On Thu, Feb 06, 2025 at 02:58:25PM +0530, Viresh Kumar wrote: > > diff --git a/MAINTAINERS b/MAINTAINERS > > index ee6709599df5..bfc1bf2ebd77 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -4021,6 +4021,7 @@ F: lib/cpumask_kunit.c > > F: lib/find_bit.c > > F: lib/find_bit_benchmark.c > > F: lib/test_bitmap.c > > +F: rust/helpers/cpumask.c > > Sorry what? I never committed to maintain this thing, and will > definitely not do it for free. > > NAK. Okay. I will add a separate entry for Rust cpumask stuff. > > +#ifndef CONFIG_CPUMASK_OFFSTACK > > +void rust_helper_free_cpumask_var(cpumask_var_t mask) > > +{ > > + free_cpumask_var(mask); > > +} > > +#endif > > This is most likely wrong because free_cpumask_var() is declared > unconditionally, and I suspect the rust helper should be as well. Non-trivial C macros and inlined C functions cannot be used directly in the Rust code and are used via functions ("helpers") that wrap those so that they can be called from Rust. The free_cpumask_var() function is defined as inline only for the CONFIG_CPUMASK_OFFSTACK=n case and that's where we need this wrapper. For the other case (CONFIG_CPUMASK_OFFSTACK=y), Rust code can directly call free_cpumask_var() from the C code and we don't need this helper. > Please for the next iteration keep me CCed for the whole series. I > want to make sure you'll not make me a rust maintainer accidentally. Sure.
+ Jason, Christoph On Tue, Feb 11, 2025 at 09:59:08AM +0530, Viresh Kumar wrote: > On 10-02-25, 19:02, Yury Norov wrote: > > On Thu, Feb 06, 2025 at 02:58:25PM +0530, Viresh Kumar wrote: > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index ee6709599df5..bfc1bf2ebd77 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -4021,6 +4021,7 @@ F: lib/cpumask_kunit.c > > > F: lib/find_bit.c > > > F: lib/find_bit_benchmark.c > > > F: lib/test_bitmap.c > > > +F: rust/helpers/cpumask.c > > > > Sorry what? I never committed to maintain this thing, and will > > definitely not do it for free. > > > > NAK. > > Okay. No, not Okay. To begin with, this is the 8th version of the same patch, but you only now bothered to CC someone who is listed in MAINTAINERS. This is not how the community works. You also made it a patch bomb that touches multiple critical and very sensitive subsystems. You link them to an experimental and unstable project, and do it in a way that makes it really easy to slip through maintainers' attention. Not speaking for others, but please for cpumasks create a separate series and start thorough discussion. > I will add a separate entry for Rust cpumask stuff. That would make things even worse. Before you wanted me to maintain rust linkage. Now you want me to get approval from someone else who maintains rust linkage. In case I need to change something, I want to be able to just change. I went deeper into the subject, and found that rust team has similar problems with other maintainers: https://lore.kernel.org/lkml/20250108122825.136021-3-abdiel.janulgue@gmail.com/ I'm particularly concerned with the following comment from Jason Gunthorpe: All PRs to Linus must not break the rust build and the responsibilty for that falls to all the maintainers. If the Rust team is not quick enough to resolve any issues during the development window then patches must be dropped before sending PRs, or Linus will refuse the PR. https://lore.kernel.org/lkml/20250130154646.GA2298732@nvidia.com/ And that happened at least once, right? I don't expect that the functions you export now will get changed anytime soon but it happens from time to time. cpumask_next_wrap() is one recent example. https://lore.kernel.org/netdev/20250128164646.4009-7-yury.norov@gmail.com/T/ The more drivers you write, the more functionality you will inevitably pull. And the risk that something will get broken one day will grow exponentially. So before we move forward, please explain in very details how would you act in a scenario described by Jason? Do you proactively run builds against development branches? If so, please add my bitmap-for-next. https://github.com/norov/linux/tree/bitmap-for-next Have you considered creating a conftest, so that if rust fails to build against the current kernel, it will get disabled until you fix the issue? Maybe you will just teach your language to understand inlines, and that's it? Does it understand macros? Thanks, Yury > > > +#ifndef CONFIG_CPUMASK_OFFSTACK > > > +void rust_helper_free_cpumask_var(cpumask_var_t mask) > > > +{ > > > + free_cpumask_var(mask); > > > +} > > > +#endif > > > > This is most likely wrong because free_cpumask_var() is declared > > unconditionally, and I suspect the rust helper should be as well. > > Non-trivial C macros and inlined C functions cannot be used directly > in the Rust code and are used via functions ("helpers") that wrap > those so that they can be called from Rust. > > The free_cpumask_var() function is defined as inline only for the > CONFIG_CPUMASK_OFFSTACK=n case and that's where we need this wrapper. > For the other case (CONFIG_CPUMASK_OFFSTACK=y), Rust code can directly > call free_cpumask_var() from the C code and we don't need this helper. > > > Please for the next iteration keep me CCed for the whole series. I > > want to make sure you'll not make me a rust maintainer accidentally. > > Sure. > > -- > viresh
On Tue, Feb 11, 2025 at 11:24:55AM -0500, Yury Norov wrote: > I'm particularly concerned with the following comment from Jason > Gunthorpe: > > All PRs to Linus must not break the rust build and the responsibilty > for that falls to all the maintainers. If the Rust team is not quick > enough to resolve any issues during the development window then > patches must be dropped before sending PRs, or Linus will refuse the > PR. > > https://lore.kernel.org/lkml/20250130154646.GA2298732@nvidia.com/ > > And that happened at least once, right? Yes, 6 patches were dropped from the last merge window by Andrew and Linus because of rust build breakage. There has since been some clarification posted: https://lore.kernel.org/all/CANiq72m-R0tOakf=j7BZ78jDHdy=9-fvZbAT8j91Je2Bxy0sFg@mail.gmail.com/ Quoting: Who is responsible if a C change breaks a build with Rust enabled? The usual kernel policy applies. So, by default, changes should not be introduced if they are known to break the build, including Rust. However, exceptionally, for Rust, a subsystem may allow to temporarily break Rust code. The intention is to facilitate friendly adoption of Rust in a subsystem without introducing a burden to existing maintainers who may be working on urgent fixes for the C side. The breakage should nevertheless be fixed as soon as possible, ideally before the breakage reaches Linus. For instance, this approach was chosen by the block laye they called it "stage 1" in their Rust integration plan. We believe this approach is reasonable as long as the kernel does not have way too many subsystems doing that (because otherwise it would be very hard to build e.g. linux-next). However, it is unclear how this "temporarily break Rust code" will work in practice. We do not know under what conditions Linus will accept a PR that forces him to go to CONFIG_RUST=n, or even if Linus has agreed to this exception policy. I suggest the safe thing for any maintainer is to not send Linus PRs that break rust, otherwise they get to be a test case to see what happens.. Jason
Hi Yury, On Tue, Feb 11, 2025 at 11:24:55AM -0500, Yury Norov wrote: > + Jason, Christoph > > On Tue, Feb 11, 2025 at 09:59:08AM +0530, Viresh Kumar wrote: > > On 10-02-25, 19:02, Yury Norov wrote: > > > On Thu, Feb 06, 2025 at 02:58:25PM +0530, Viresh Kumar wrote: > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > index ee6709599df5..bfc1bf2ebd77 100644 > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -4021,6 +4021,7 @@ F: lib/cpumask_kunit.c > > > > F: lib/find_bit.c > > > > F: lib/find_bit_benchmark.c > > > > F: lib/test_bitmap.c > > > > +F: rust/helpers/cpumask.c > > > > > > Sorry what? I never committed to maintain this thing, and will > > > definitely not do it for free. > > > > > > NAK. > > > > Okay. > > No, not Okay. > > To begin with, this is the 8th version of the same patch, but you > only now bothered to CC someone who is listed in MAINTAINERS. This is > not how the community works. I'm answering, since I was involved in the discussion you refer to below, but please also let me add a few other thoughts from my side. First of all, I think you are right here. AFAICS, the cpumask abstraction was added to this series in v6, and you were CC'd in v8, which is *not* OK. I also agree that this definitely deserves it's own series for you to be properly involved. > > I will add a separate entry for Rust cpumask stuff. > > That would make things even worse. Before you wanted me to maintain > rust linkage. Now you want me to get approval from someone else who > maintains rust linkage. In case I need to change something, I want to > be able to just change. I think the decision is up to you, whether 1) You want to maintain it on your own. 2) You want a co-maintainer / reviewer that takes care of the Rust parts. 3) You want nothing to do with it and have it maintained as a separate component. In case you prefer option 3), please do *not* see it as "you need to get approval from someone else for code changes in your subsystem", because no ones wants to impose this on you. Please see it as just another driver or another component that makes use of the API you maintain. If you are running into API changes that affect the Rust abstraction, that's where you can refer to the maintainer of the Rust abstraction to help out. Just like with every other driver or component that uses a core API, which isn't trivial to adjust. > > I went deeper into the subject, and found that rust team has similar > problems with other maintainers: > > https://lore.kernel.org/lkml/20250108122825.136021-3-abdiel.janulgue@gmail.com/ I don't think that this case is similar to the one you linked in. I think you were indeed a bit bypassed here, plus you seem to have a real concern with maintainership, which I think is fair to be addressed and resolved. I hope my reply already helps with that. - Danilo
On Tue, Feb 11, 2025 at 5:24 PM Yury Norov <yury.norov@gmail.com> wrote: > > No, not Okay. > > To begin with, this is the 8th version of the same patch, but you > only now bothered to CC someone who is listed in MAINTAINERS. This is > not how the community works. Yeah, that is not good. For what it is worth, we try to make this very clear: https://rust-for-linux.com/contributing#submitting-new-abstractions-and-modules i.e. that maintainers need to be Cc'd and contacted as soon as possible (possibly even before writing the code). > You also made it a patch bomb that touches multiple critical and very > sensitive subsystems. You link them to an experimental and unstable > project, and do it in a way that makes it really easy to slip through > maintainers' attention. Not sure what you mean by "unstable project", but I agree that the patch series, unless Viresh is the maintainer of the C side of everything added, it should be discussed and maintenance discussed accordingly before merging anything. This is what we have done for everything else, and that has not changed. I try to spot cases where this is not done, which is why I Cc'd you in v7 and told Viresh to please do so, and he did -- I don't think he was trying to bypass on purpose: https://lore.kernel.org/rust-for-linux/CANiq72=o+uc3ZnNrdkuoSGSL8apNE4z4QwpvsiLfGzXFywSLrQ@mail.gmail.com/ > That would make things even worse. Before you wanted me to maintain > rust linkage. Now you want me to get approval from someone else who > maintains rust linkage. In case I need to change something, I want to > be able to just change. Like Danilo mentions, there are several ways to go forward here. For some ideas/context, please see: https://rust-for-linux.com/rust-kernel-policy#how-is-rust-introduced-in-a-subsystem (Thanks Jason for linking the page) And, yeah, whoever ends up maintaining this, then they should of course be testing it properly with Rust enabled with a proper config for that and so on, just like one would do for anything else. By the way, it is possible to request Intel's 0day to build with Rust enabled. (Side-note: to clarify, there are different parties involved here -- "Rust team" is fairly ambiguous.) Cheers, Miguel
On 11-02-25, 11:24, Yury Norov wrote: > To begin with, this is the 8th version of the same patch, but you > only now bothered to CC someone who is listed in MAINTAINERS. This is > not how the community works. Yes, this was my mistake. I assumed get_maintainers would Cc all relevant people, but I overlooked the fact that these are new files, so it didn’t include the maintainers. Unfortunately, the same issue occurred with the clk bindings. Miguel pointed it out at V7, and I corrected it immediately. There was no intention to bypass anyone. These bindings had been in my tree since earlier versions, but I hesitated to post them before V6 because I wasn’t confident in writing bindings for a framework I didn’t fully understand. I eventually shared them in V6 to unblock my series but inadvertently missed Cc’ing few, as mentioned above. > You also made it a patch bomb that touches multiple critical and very > sensitive subsystems. You link them to an experimental and unstable > project, and do it in a way that makes it really easy to slip through > maintainers' attention. > > Not speaking for others, but please for cpumasks create a separate > series and start thorough discussion. Agree, its better to post these separately.
Hi Miguel, On Tue, Feb 11, 2025 at 10:37:26PM +0100, Miguel Ojeda wrote: > On Tue, Feb 11, 2025 at 5:24 PM Yury Norov <yury.norov@gmail.com> wrote: > > > > No, not Okay. > > > > To begin with, this is the 8th version of the same patch, but you > > only now bothered to CC someone who is listed in MAINTAINERS. This is > > not how the community works. > > Yeah, that is not good. > > For what it is worth, we try to make this very clear: > > https://rust-for-linux.com/contributing#submitting-new-abstractions-and-modules > > i.e. that maintainers need to be Cc'd and contacted as soon as > possible (possibly even before writing the code). > > > You also made it a patch bomb that touches multiple critical and very > > sensitive subsystems. You link them to an experimental and unstable > > project, and do it in a way that makes it really easy to slip through > > maintainers' attention. > > Not sure what you mean by "unstable project", but I agree that the I mean that Andrew's branch got broken because of it. > patch series, unless Viresh is the maintainer of the C side of > everything added, it should be discussed and maintenance discussed > accordingly before merging anything. > > This is what we have done for everything else, and that has not changed. > > I try to spot cases where this is not done, which is why I Cc'd you in > v7 and told Viresh to please do so, and he did -- I don't think he was > trying to bypass on purpose: > > https://lore.kernel.org/rust-for-linux/CANiq72=o+uc3ZnNrdkuoSGSL8apNE4z4QwpvsiLfGzXFywSLrQ@mail.gmail.com/ Yes, I see. Viresh didn't do that on purpose. Let's move forward. I'm more concerned about lack of testing on rust side that ended up with the patches withdrawal. > > That would make things even worse. Before you wanted me to maintain > > rust linkage. Now you want me to get approval from someone else who > > maintains rust linkage. In case I need to change something, I want to > > be able to just change. > > Like Danilo mentions, there are several ways to go forward here. For > some ideas/context, please see: > > https://rust-for-linux.com/rust-kernel-policy#how-is-rust-introduced-in-a-subsystem > > (Thanks Jason for linking the page) > > And, yeah, whoever ends up maintaining this, then they should of > course be testing it properly with Rust enabled with a proper config > for that and so on, just like one would do for anything else. By the > way, it is possible to request Intel's 0day to build with Rust > enabled. Yes, this should be done. 0-day is already testing everything in my https://github.com/norov/linux repo. If you know right people, please ask them to test bitmap-for-next branch with rust enabled. If there's enough resources, please cover all the repo. > (Side-note: to clarify, there are different parties involved here -- > "Rust team" is fairly ambiguous.) > > Cheers, > Miguel I'm the right person to look after rust bindings for bitops, inevitably. I will take over patch 4/14 and submit it separately together with a new maintenance entry. I will not maintain any rust code. For 5/14, after I'll send my series, Viresh, can you submit 5/14 separately and create a separate entry in MAINTAINERS. Please make me a reviewer there. I'll think about details over the weekend and will submit everything early next week. Rasmus, if you would like to help me reviewing this thing, please let me know. Thanks, Yury
On 13-02-25, 21:20, Yury Norov wrote: > I'm the right person to look after rust bindings for bitops, inevitably. > I will take over patch 4/14 and submit it separately together with a > new maintenance entry. > > I will not maintain any rust code. For 5/14, after I'll send my series, > Viresh, can you submit 5/14 separately and create a separate entry in > MAINTAINERS. Please make me a reviewer there. Sounds good. Thanks.
On Fri, Feb 14, 2025 at 3:20 AM Yury Norov <yury.norov@gmail.com> wrote: > > I mean that Andrew's branch got broken because of it. (This response is extra verbose to be extra clear, given the recent discussions these last days -- I don't mean you said or implied any of this, sorry) To be clear: it was not a patch we introduced that broke it. It was a patch series from someone else that got a Linaro build report and that, nevertheless, was sent to Linus. The build report reached our mailing list, but we were not Cc'd directly about it (i.e. as individuals), we were not pinged about it (i.e. we never had any further questions about that after that single day), and we didn't realize the series was kept in a queue targeting Linus. Moreover, the build failure happened with a configuration that is best-effort and is documented as very experimental (GCC mixed builds), and that we didn't know Linus was building (we knew he built with Clang+Rust, which is what we are focused on testing), so we didn't spot that either on our side. By the way, we have not been Cc'd in the new version, either... Nor the mailing list. To be clear, I am not blaming anyone for the breakage, and I already apologized that we didn't notice that report. But it is not our fault either. We also never promised we would fix every single Rust issue spotted across the entire kernel. We try to do our best to help, though. https://rust-for-linux.com/rust-kernel-policy#who-is-responsible-if-a-c-change-breaks-a-build-with-rust-enabled https://rust-for-linux.com/rust-kernel-policy#didnt-you-promise-rust-wouldnt-be-extra-work-for-maintainers > Yes, I see. Viresh didn't do that on purpose. Let's move forward. I'm > more concerned about lack of testing on rust side that ended up with > the patches withdrawal. Please see above. I regularly test different combinations (branches, configs, compiler versions, and so on) to catch mainly toolchain issues and so on, and keep things as clean as I can. Others use regularly the Rust support for their different use cases, thus more testing happens on different environments. In other words, things generally work just fine. However, our testing is not meant to catch every issue everywhere. Like for anything else in the kernel, whoever maintains a branch with a particular Rust feature needs to set up proper testing for that particular feature and relevant configs. Regarding the GCC mixed builds: worst case, if we see there is no bandwidth for it (be it ourselves or other maintainers testing their own branches), we could limit the testing required (e.g. limiting GCC versions when Rust is enabled), or we could request Linus to skip Rust with GCC in his builds, so that at least PRs are not blocked on that. > Yes, this should be done. 0-day is already testing everything in my > https://github.com/norov/linux repo. If you know right people, please > ask them to test bitmap-for-next branch with rust enabled. If there's > enough resources, please cover all the repo. Definitely! I will do that and Cc you. I hope that helps. > I'm the right person to look after rust bindings for bitops, inevitably. > I will take over patch 4/14 and submit it separately together with a > new maintenance entry. > > I will not maintain any rust code. For 5/14, after I'll send my series, > Viresh, can you submit 5/14 separately and create a separate entry in > MAINTAINERS. Please make me a reviewer there. > > I'll think about details over the weekend and will submit everything > early next week. Thanks a lot Yury, that sounds very reasonable. Cheers, Miguel
On Fri, Feb 14, 2025 at 06:56:43PM +0100, Miguel Ojeda wrote: > > I mean that Andrew's branch got broken because of it. > We also never promised we would fix every single Rust issue spotted > across the entire kernel. We try to do our best to help, though. Sure, but it was said, by many people, many times, that "Rust is allowed to break". This is not just my incorrect impression. For instance read Philipp's note to Christoph: https://lore.kernel.org/all/293df3d54bad446e8fd527f204c6dc301354e340.camel@mailbox.org/ > reassure you that the burden of keeping Rust abstractions working with > any changes on the C side rests entirely on the Rust side's shoulders? > (because that's what the statements made by the latter seem to mean to > me) And Greg's version: https://lore.kernel.org/all/2025013030-gummy-cosmic-7927@gregkh/ > So the claim remains the same here. It's just like staging, api changes > to subsystems are allowed to break staging, and rust code, and > maintainers do NOT have to fix them up there, that's up to the staging > and rust maintainers/developers to do so. I've heard the same statements at conferences and in other coverages like LWN. Frankly, I never much believed in this story as workable, but it was advanced by many people to smooth the adoption of Rust bindings. > https://rust-for-linux.com/rust-kernel-policy#didnt-you-promise-rust-wouldnt-be-extra-work-for-maintainers I do not agree with "Didn't you promise Rust wouldn't be extra work for maintainers?" in your document. Clearly there is a widespread belief this kind of promise was made, even if it was never made by you. "Rust is allowed to break" is understood to be the same as saying it won't cause extra work. However, I am glad we are seeing a more realistic understanding of what Rust requires of the community over the long term. Jason
On Fri, Feb 14, 2025 at 8:11 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > Sure, but it was said, by many people, many times, that "Rust is > allowed to break". A lot of people have said many things (especially in online fora), and many of those things are contradictory, but that does not really mean anything. > This is not just my incorrect impression. For instance read Philipp's > note to Christoph: > > https://lore.kernel.org/all/293df3d54bad446e8fd527f204c6dc301354e340.camel@mailbox.org/ Philipp probably sent that message with his best intentions, but he is not part of the Rust subsystem nor speaks on our behalf. He may have been speaking for other groups, or maybe just his own opinion -- I do not know. Moreover, sometimes suggestions like his may be referring to particular subsystems (like the one that was discussed in that thread), e.g. like block decided to take an approach where Rust is allowed to break, rather than speaking globally. Finally, ambiguous terms are used in many cases to refer to different parties: "Rust community", "Rust people", "Rust team", "Rust maintainers"... I have started to ask people to avoid doing that (at least in the LKML), please, and be concrete if possible. > And Greg's version: > > https://lore.kernel.org/all/2025013030-gummy-cosmic-7927@gregkh/ I cannot speak for Greg, sorry. I can read his message in the following ways: - I can read it as a general ability of subsystems to potentially agree to treat Rust code as something like staging, like block's plan. - I can read it within the context of those patches, where, as far as I know, Danilo et al. stepped up to maintain it, like Andreas did for block. - I can read it as the fact that the Rust subsystem will help, best-effort, to bootstrap Rust and help with integration where possible. We need maintainers' help and expertise from other subsystems to succeed. And we do not want to force other subsystems into dealing with Rust. That is why the deal was that we would contact and wait for other subsystems to handle Rust and so on. It is also why I asked, in the very meeting where it was decided to merge Rust, that in exchange, we would eventually need some flexibility by maintainers that may not want Rust in their subsystem but that nevertheless may be the owners of core APIs that other users of Rust in the kernel need. I did so because I knew the day would come we would be in the situation we are in that email thread. > I've heard the same statements at conferences and in other coverages > like LWN. Frankly, I never much believed in this story as workable, > but it was advanced by many people to smooth the adoption of Rust > bindings. Again, people may make statements, but they may be local to their subsystem, or just their opinion, or it may be a misunderstanding, and so on and so forth. It is very hard to keep hundreds of maintainers on the same page. > I do not agree with "Didn't you promise Rust wouldn't be extra work > for maintainers?" in your document. Clearly there is a widespread > belief this kind of promise was made, even if it was never made by > you. "Rust is allowed to break" is understood to be the same as saying > it won't cause extra work. Sorry, but I have to strongly push back against this paragraph. Are you really saying that, because people out there may think something, we cannot claim anymore that we did not promise something? Furthermore, I don't agree with your assessment in your last sentence at all. Even if it was decided to allow Rust to break globally and at all times, it does not mean Rust is not extra work. It is, because collaboration would be still needed with different subsystems and so on. The plan has always been to not have Rust be something hidden in a corner. For instance, see from 2020: https://lore.kernel.org/all/CAHk-=wipXqemHbVnK1kQsFzGOOZ8FUXn3PKrZb5WC=KkgAjRRw@mail.gmail.com/ > However, I am glad we are seeing a more realistic understanding of > what Rust requires of the community over the long term. That is good, but to be clear, from my point of view, the approach mentioned in the document I wrote is what we have always said. Cheers, Miguel
On Fri, Feb 14, 2025 at 8:11 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > Philipp > Greg Since they were mentioned by name, we should Cc them out of courtesy. Cheers, Miguel
On Fri, Feb 14, 2025 at 09:24:31PM +0100, Miguel Ojeda wrote: > On Fri, Feb 14, 2025 at 8:11 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > Sure, but it was said, by many people, many times, that "Rust is > > allowed to break". > > A lot of people have said many things (especially in online fora), and > many of those things are contradictory, but that does not really mean > anything. We don't have a community where there is a clear authority structure. If lots of people are repeating a thing, that thing usually becomes the consensus and common viewpoint regardless of who originated it. The repetition reflects community agreement and buy in of the idea. At the end of the day, only the policy adopted by the people merging patches really matters. The assumption being if respected people speak with authority on a policy they have also got buy in from the people responsible to execute it. I also think you should merge your policy document to the tree, not keep it on a web page. That seems to be a big part of getting community agreed policy adopted. > Finally, ambiguous terms are used in many cases to refer to different > parties: "Rust community", "Rust people", "Rust team", "Rust > maintainers"... I have started to ask people to avoid doing that (at > least in the LKML), please, and be concrete if possible. Okay, that makes lots of seense. Please don't use "we" as well.. I have no idea who is included in your "we", or what to even call it. > > And Greg's version: > > > > https://lore.kernel.org/all/2025013030-gummy-cosmic-7927@gregkh/ > > I cannot speak for Greg, sorry. Yet he does seems to be speaking with authority on this topic. His message was quoted on LWN and likely was read by a large number of maintainers. Is he not part of your "we"? > I can read his message in the following ways: I think it was very clear, sorry, I don't read it your many ways at all. > - I can read it as a general ability of subsystems to potentially > agree to treat Rust code as something like staging, like block's plan. As a side note, I don't see how anyone can enact this plan without the support of Linus to do CONFIG_RUST=n builds and put out a non-working rc1. IMHO it is yet unclear if this is real thing or an unproven idea block has that will run into problems. > It is very hard to keep hundreds of maintainers on the same page. It is, but also I think you need to take this challenge to succeed. > > I do not agree with "Didn't you promise Rust wouldn't be extra work > > for maintainers?" in your document. Clearly there is a widespread > > belief this kind of promise was made, even if it was never made by > > you. "Rust is allowed to break" is understood to be the same as saying > > it won't cause extra work. > > Sorry, but I have to strongly push back against this paragraph. > > Are you really saying that, because people out there may think > something, we cannot claim anymore that we did not promise something? Again "we" ? I'm not concerned about "people out there". Greg said it. Others who I would think are part of your "we" have posted it on LKML. It is not clear at all. If you said Miguel never claimed it, then I would not complain. You said it correctly above, be concrete. Ideally acknowledge there were different policy ideas in wide circulation, but they did not get adopted. > Furthermore, I don't agree with your assessment in your last sentence > at all. Even if it was decided to allow Rust to break globally and at > all times, it does not mean Rust is not extra work. I appreciate this point is realistically true, but look at how Philipp presented this 'no break' concept to Christoph as a no-work option for him. My main point here is that I'm glad things are getting straightened out, some of the conflicting policy ideas shot down, and the demands on maintainers are being articulated more clearly. > That is good, but to be clear, from my point of view, the approach > mentioned in the document I wrote is what we have always said. There is another we :) Jason
On Fri, Feb 14, 2025 at 10:06 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > We don't have a community where there is a clear authority > structure. If lots of people are repeating a thing, that thing usually > becomes the consensus and common viewpoint regardless of who > originated it. The repetition reflects community agreement and buy in > of the idea. We cannot be expected to chase down every maintainer in all online fora. Not just because it is not viable, but because hopefully this is not about who shouts the loudest (== more often). Moreover, we do not have the authority to set policy ourselves, and we are not in the business of forcing things into other subsystems. Now, if you are a maintainer, and you want to deal with Rust in the kernel, then please do any of the following: send us an email, join our talks, read the LWN articles about our talks (but do not assume LWN comments are "consensus" somehow), join our MC in LPC, join Kangrejos, and so on and so forth. We have been open to talk to every single maintainer that wanted to use Rust or that we needed to interface with, and other maintainers can tell you that we have successfully worked with them. > At the end of the day, only the policy adopted by the people merging > patches really matters. > > The assumption being if respected people speak with authority on a > policy they have also got buy in from the people responsible to > execute it. Sure, and that is why we talked to the actual, relevant, maintainers and subsystems that are taking patches that are related to Rust. We may not have talked to you directly or explain things to you, but that does not mean we didn't talk to others. > I also think you should merge your policy document to the tree, not > keep it on a web page. That seems to be a big part of getting > community agreed policy adopted. Very happy to do so if others are happy with it. I published it in the website because it is not a document the overall kernel community signed on so far. Again, we do not have that authority as far as I understand. The idea was to clarify the main points, and gather consensus. The FOSDEM 2025 keynote quotes were also intended in a similar way: https://fosdem.org/2025/events/attachments/fosdem-2025-6507-rust-for-linux/slides/236835/2025-02-0_iwSaMYM.pdf > Okay, that makes lots of seense. Please don't use "we" as well.. I > have no idea who is included in your "we", or what to even call it. In my emails in this thread, "we" means the Rust subsystem entry. > Yet he does seems to be speaking with authority on this topic. His > message was quoted on LWN and likely was read by a large number of > maintainers. > > Is he not part of your "we"? No, he is not part of the Rust subsystem entry, as you can easily verify in the `MAINTAINERS` file. And I can not speak for him either. He, of course, has helped us a lot, like other kernel maintainers have. By the way, the document, at the top, mentions: Like most things in the kernel, these points are not hard rules and can change over time depending on the situation and what key maintainers and the kernel community discuss. > I think it was very clear, sorry, I don't read it your many ways at > all. Well, then please ask Greg, not us, and remember to Cc him, by the way. > As a side note, I don't see how anyone can enact this plan without the > support of Linus to do CONFIG_RUST=n builds and put out a non-working > rc1. IMHO it is yet unclear if this is real thing or an unproven idea > block has that will run into problems. Please ask Jens and the block layer -- Cc'ing Jens (Andreas and Boqun are already Cc'd): https://lore.kernel.org/all/593a98c9-baaa-496b-a9a7-c886463722e1@kernel.dk/ Having said that, I am not sure what you mean by -rc1. It is in the context of a friendly collaboration -- I assume the intention is that Andreas et al. are given enough lead time on new features to fix them before the merge window. For fixes, it may be harder, of course. Other ideas: they may be able to config out certain parts too; or in the worst case, in an emergency, Linus may decide to break Rust. They may be able to tell you the details of their plan. > It is, but also I think you need to take this challenge to succeed. In fact, we are, and I explained above all the ways we are engaging with maintainers and so on. Please note that the fact that we may not tackle the challenge in the way you would like does not mean we are not doing it. > It is not clear at all. If you said Miguel never claimed it, then I > would not complain. You said it correctly above, be concrete. Ideally > acknowledge there were different policy ideas in wide circulation, but > they did not get adopted. We have never (intentionally) claimed that. We may have spoken unclearly, we may have been misinterpreted by others, and we have always been open for clarification. Please see my previous email. > I appreciate this point is realistically true, but look at how Philipp > presented this 'no break' concept to Christoph as a no-work option for > him. > > My main point here is that I'm glad things are getting straightened > out, some of the conflicting policy ideas shot down, and the demands > on maintainers are being articulated more clearly. Again, to be clear: the approach we have followed has been quite clear if you actually spoke to us, instead of taking for granted what unrelated people may have posted. > There is another we :) I am not sure what you are trying to achieve here. Cheers, Miguel
"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes: > On Fri, Feb 14, 2025 at 10:06 PM Jason Gunthorpe <jgg@nvidia.com> wrote: [...] >> As a side note, I don't see how anyone can enact this plan without the >> support of Linus to do CONFIG_RUST=n builds and put out a non-working >> rc1. IMHO it is yet unclear if this is real thing or an unproven idea >> block has that will run into problems. > > Please ask Jens and the block layer -- Cc'ing Jens (Andreas and Boqun > are already Cc'd): > > https://lore.kernel.org/all/593a98c9-baaa-496b-a9a7-c886463722e1@kernel.dk/ > > Having said that, I am not sure what you mean by -rc1. It is in the > context of a friendly collaboration -- I assume the intention is that > Andreas et al. are given enough lead time on new features to fix them > before the merge window. For fixes, it may be harder, of course. Other > ideas: they may be able to config out certain parts too; or in the > worst case, in an emergency, Linus may decide to break Rust. They may > be able to tell you the details of their plan. Maybe I can help move the discussion forward by describing how we do things in block. In block we (the block subsystem community) currently have the rule that rust code should not delay shipping a PR. I am not sure how Jens will enforce this, but I could imagine that if builds start failing by the time a PR has to be submitted, Jens would just yank rust block code. And so no fallout of this would reach Linus. Of course, there may be situations where problems do not surface until Linux is merging things, but for this to happen without these issues first appearing in linux-next would be extremely unlikely. Maybe having a config option to disable rust block might be a good idea, to prevent the yanking if it ever comes to that. In practice, we never had any issues. Things have broken a handful of times, but I usually see it within 24 ours and then I am able to send a fix. I can't imagine that Jens has been forced to spend a lot of cycles on this, outside of applying a few fixes now and again. It would be interesting to know how much the workload has actually been for him. Anyways, it is my hope that within a few years, rust will become a fully qualified citizen in block, and the special rule can be dropped. This of course requires Jens becoming able and willing to handle rust related issues himself, or him becoming confident that the current arrangement will suffice for solving any rust related issues. Best regards, Andreas Hindborg
"Viresh Kumar" <viresh.kumar@linaro.org> writes: > On 11-02-25, 11:24, Yury Norov wrote: >> To begin with, this is the 8th version of the same patch, but you >> only now bothered to CC someone who is listed in MAINTAINERS. This is >> not how the community works. > > Yes, this was my mistake. I assumed get_maintainers would Cc all > relevant people, but I overlooked the fact that these are new files, > so it didn’t include the maintainers. Unfortunately, the same issue > occurred with the clk bindings. Miguel pointed it out at V7, and I > corrected it immediately. There was no intention to bypass anyone. I have to admit, I have done this as well, for the module_params series. It feels horrible to commit this action, because it looks either disrespectful or like an attempt to bypass people to sneak in stuff. But as clarified this is _never_ the intention. It's simply people working with a contribution model that they are still learning how to use. The kernel tooling and process is not exactly intuitive to most people. Just know that absolutely the largest fraction of contributors (I would guess all) want to do things the right way. For all contributions, rust or c, the patches benefit tremendously from reviews from domain experts. The contributors appreciate this and hope for this to happen. Best regards, Andreas Hindborg
On Fri, 2025-02-14 at 17:06 -0400, Jason Gunthorpe wrote: > On Fri, Feb 14, 2025 at 09:24:31PM +0100, Miguel Ojeda wrote: > > On Fri, Feb 14, 2025 at 8:11 PM Jason Gunthorpe <jgg@nvidia.com> > > wrote: > > > > > > Sure, but it was said, by many people, many times, that "Rust is > > > allowed to break". > > > > A lot of people have said many things (especially in online fora), > > and > > many of those things are contradictory, but that does not really > > mean > > anything. > > We don't have a community where there is a clear authority > structure. If lots of people are repeating a thing, that thing > usually > becomes the consensus and common viewpoint regardless of who > originated it. The repetition reflects community agreement and buy in > of the idea. > > At the end of the day, only the policy adopted by the people merging > patches really matters. > > The assumption being if respected people speak with authority on a > policy they have also got buy in from the people responsible to > execute it. > > I also think you should merge your policy document to the tree, not > keep it on a web page. That seems to be a big part of getting > community agreed policy adopted. > > > Finally, ambiguous terms are used in many cases to refer to > > different > > parties: "Rust community", "Rust people", "Rust team", "Rust > > maintainers"... I have started to ask people to avoid doing that > > (at > > least in the LKML), please, and be concrete if possible. > > Okay, that makes lots of seense. Please don't use "we" as well.. I > have no idea who is included in your "we", or what to even call it. > > > > And Greg's version: > > > > > > https://lore.kernel.org/all/2025013030-gummy-cosmic-7927@gregkh/ > > > > I cannot speak for Greg, sorry. > > Yet he does seems to be speaking with authority on this topic. His > message was quoted on LWN and likely was read by a large number of > maintainers. > > Is he not part of your "we"? > > > I can read his message in the following ways: > > I think it was very clear, sorry, I don't read it your many ways at > all. > > > - I can read it as a general ability of subsystems to potentially > > agree to treat Rust code as something like staging, like block's > > plan. > > As a side note, I don't see how anyone can enact this plan without > the > support of Linus to do CONFIG_RUST=n builds and put out a non-working > rc1. IMHO it is yet unclear if this is real thing or an unproven idea > block has that will run into problems. > > > It is very hard to keep hundreds of maintainers on the same page. > > It is, but also I think you need to take this challenge to succeed. > > > > I do not agree with "Didn't you promise Rust wouldn't be extra > > > work > > > for maintainers?" in your document. Clearly there is a widespread > > > belief this kind of promise was made, even if it was never made > > > by > > > you. "Rust is allowed to break" is understood to be the same as > > > saying > > > it won't cause extra work. > > > > Sorry, but I have to strongly push back against this paragraph. > > > > Are you really saying that, because people out there may think > > something, we cannot claim anymore that we did not promise > > something? > > Again "we" ? > > I'm not concerned about "people out there". Greg said it. Others who > I > would think are part of your "we" have posted it on LKML. > > It is not clear at all. If you said Miguel never claimed it, then I > would not complain. You said it correctly above, be concrete. Ideally > acknowledge there were different policy ideas in wide circulation, > but > they did not get adopted. > > > Furthermore, I don't agree with your assessment in your last > > sentence > > at all. Even if it was decided to allow Rust to break globally and > > at > > all times, it does not mean Rust is not extra work. > > I appreciate this point is realistically true, but look at how > Philipp > presented this 'no break' concept to Christoph as a no-work option > for > him. Hello, so to clarify that: I'm (currently) not writing or maintaining any Rust abstractions. So I'm not representing official projects, so take the statement for what it's worth. I mainly wanted to probe how an agreement with Christoph could have been worked out. My main point still holds up, though: The rules should be written down. In tree, in linux/ As you, Jason, also state, this would enforce a discussion leading to more clarity. > > My main point here is that I'm glad things are getting straightened > out, some of the conflicting policy ideas shot down, and the demands > on maintainers are being articulated more clearly. > > > That is good, but to be clear, from my point of view, the approach > > mentioned in the document I wrote is what we have always said. > > There is another we :) The current, apparent, distinction between Rust and non-Rust kernel engineers might indeed be a symbol for an underlying big question. 10 years down the road, will there still be "Rust people" who take care of… the compiler? The abstractions and bindings? Are there "Clang people and GCC people" currently, too? That question goes deeper than one might think, because it's ultimately about who maintains what and, as we already discussed, who can break what. If, in the mid-future, subsystem maintainers uninvolved with Rust couldn't merge changes that break Rust anymore, that *might* de facto indeed lead to them having to learn that language. Or everyone would need a Rust-proficient co-maintainer who can deal with those breakages; or similar solutions. So that's why I think clarifying it rather sooner than later is necessary. P. > > Jason >
diff --git a/MAINTAINERS b/MAINTAINERS index ee6709599df5..bfc1bf2ebd77 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4021,6 +4021,7 @@ F: lib/cpumask_kunit.c F: lib/find_bit.c F: lib/find_bit_benchmark.c F: lib/test_bitmap.c +F: rust/helpers/cpumask.c F: tools/include/linux/bitfield.h F: tools/include/linux/bitmap.h F: tools/include/linux/bits.h diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index fda1e0d8f376..59b4bc49d039 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -11,6 +11,7 @@ #include <linux/blk_types.h> #include <linux/blkdev.h> #include <linux/cpu.h> +#include <linux/cpumask.h> #include <linux/cred.h> #include <linux/errname.h> #include <linux/ethtool.h> diff --git a/rust/helpers/cpumask.c b/rust/helpers/cpumask.c new file mode 100644 index 000000000000..49267ad33b3c --- /dev/null +++ b/rust/helpers/cpumask.c @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/cpumask.h> + +void rust_helper_cpumask_set_cpu(unsigned int cpu, struct cpumask *dstp) +{ + cpumask_set_cpu(cpu, dstp); +} + +void rust_helper_cpumask_clear_cpu(int cpu, struct cpumask *dstp) +{ + cpumask_clear_cpu(cpu, dstp); +} + +void rust_helper_cpumask_setall(struct cpumask *dstp) +{ + cpumask_setall(dstp); +} + +unsigned int rust_helper_cpumask_weight(struct cpumask *srcp) +{ + return cpumask_weight(srcp); +} + +void rust_helper_cpumask_copy(struct cpumask *dstp, const struct cpumask *srcp) +{ + cpumask_copy(dstp, srcp); +} + +bool rust_helper_zalloc_cpumask_var(cpumask_var_t *mask, gfp_t flags) +{ + return zalloc_cpumask_var(mask, flags); +} + +#ifndef CONFIG_CPUMASK_OFFSTACK +void rust_helper_free_cpumask_var(cpumask_var_t mask) +{ + free_cpumask_var(mask); +} +#endif diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index 0640b7e115be..de2341cfd917 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c @@ -11,6 +11,7 @@ #include "bug.c" #include "build_assert.c" #include "build_bug.c" +#include "cpumask.c" #include "cred.c" #include "device.c" #include "err.c"
In order to prepare for adding Rust abstractions for cpumask, this patch adds cpumask helpers. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- MAINTAINERS | 1 + rust/bindings/bindings_helper.h | 1 + rust/helpers/cpumask.c | 40 +++++++++++++++++++++++++++++++++ rust/helpers/helpers.c | 1 + 4 files changed, 43 insertions(+) create mode 100644 rust/helpers/cpumask.c