[PATCH] rust: id_pool: fix example

[PATCH] rust: id_pool: fix example

[Miguel Ojeda]

When building with KUnit doctests enabled, `rustc` reports:

    error[E0599]: no method named `acquire_next_id` found for struct `IdPool` in the current scope
        --> rust/doctests_kernel_generated.rs:6722:24
         |
    6722 |     assert_eq!(i, pool.acquire_next_id(i).ok_or(ENOSPC)?);
         |                        ^^^^^^^^^^^^^^^ method not found in `IdPool`

Thus fix it.

Fixes: a5726454470c ("rust: id_pool: do not immediately acquire new ids")
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
I saw this in -next.

 rust/kernel/id_pool.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
index 73a952d7dd83..384753fe0e44 100644
--- a/rust/kernel/id_pool.rs
+++ b/rust/kernel/id_pool.rs
@@ -28,7 +28,7 @@
 ///
 /// let mut pool = IdPool::with_capacity(64, GFP_KERNEL)?;
 /// for i in 0..64 {
-///     assert_eq!(i, pool.acquire_next_id(i).ok_or(ENOSPC)?);
+///     assert_eq!(i, pool.find_unused_id(i).ok_or(ENOSPC)?.acquire());
 /// }
 ///
 /// pool.release_id(23);

base-commit: 00c5ce039598e692e1dd4bf2b3ad5bc08bdf3270
--
2.52.0

Re: [PATCH] rust: id_pool: fix example

[Alice Ryhl]

On Mon, Dec 01, 2025 at 01:09:49AM +0100, Miguel Ojeda wrote:
> When building with KUnit doctests enabled, `rustc` reports:
> 
>     error[E0599]: no method named `acquire_next_id` found for struct `IdPool` in the current scope
>         --> rust/doctests_kernel_generated.rs:6722:24
>          |
>     6722 |     assert_eq!(i, pool.acquire_next_id(i).ok_or(ENOSPC)?);
>          |                        ^^^^^^^^^^^^^^^ method not found in `IdPool`
> 
> Thus fix it.
> 
> Fixes: a5726454470c ("rust: id_pool: do not immediately acquire new ids")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH] rust: id_pool: fix example

[Yury Norov]

On Mon, Dec 01, 2025 at 01:09:49AM +0100, Miguel Ojeda wrote:
> When building with KUnit doctests enabled, `rustc` reports:
> 
>     error[E0599]: no method named `acquire_next_id` found for struct `IdPool` in the current scope
>         --> rust/doctests_kernel_generated.rs:6722:24
>          |
>     6722 |     assert_eq!(i, pool.acquire_next_id(i).ok_or(ENOSPC)?);
>          |                        ^^^^^^^^^^^^^^^ method not found in `IdPool`
> 
> Thus fix it.
> 
> Fixes: a5726454470c ("rust: id_pool: do not immediately acquire new ids")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Thanks Miguel,

I applied this, but the fact that you've sent a second fix to
documentation that actually is a build fix, raises the questions.

Because Rust documentation bears compilable chunks of code, I think
we need to enable rustdoc tests target by default, so that developers
will not send broken tests.

Thanks,
Yury

> ---
> I saw this in -next.
> 
>  rust/kernel/id_pool.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
> index 73a952d7dd83..384753fe0e44 100644
> --- a/rust/kernel/id_pool.rs
> +++ b/rust/kernel/id_pool.rs
> @@ -28,7 +28,7 @@
>  ///
>  /// let mut pool = IdPool::with_capacity(64, GFP_KERNEL)?;
>  /// for i in 0..64 {
> -///     assert_eq!(i, pool.acquire_next_id(i).ok_or(ENOSPC)?);
> +///     assert_eq!(i, pool.find_unused_id(i).ok_or(ENOSPC)?.acquire());
>  /// }
>  ///
>  /// pool.release_id(23);
> 
> base-commit: 00c5ce039598e692e1dd4bf2b3ad5bc08bdf3270
> --
> 2.52.0

Re: [PATCH] rust: id_pool: fix example

[Miguel Ojeda]

On Mon, Dec 1, 2025 at 8:29 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> Thanks Miguel,
>
> I applied this, but the fact that you've sent a second fix to
> documentation that actually is a build fix, raises the questions.
>
> Because Rust documentation bears compilable chunks of code, I think
> we need to enable rustdoc tests target by default, so that developers
> will not send broken tests.

You're welcome!

This one is a Kconfig in the normal build, i.e.
`CONFIG_RUST_KERNEL_DOCTESTS` (it is true that the other patch was for
a different target).

If you mean enabling that by Kconfig default, then I don't think we
can do that -- KUnit on its own (which this uses) is not meant for
normal builds. Perhaps we could have something that just checks the
build, but people should really run the doctests anyway, since they
typically contain `assert!`s and so on that can be wrong.

If you mean enabling it in Intel's kernel test robot, then I think
they do it (or at least I told them about it long ago). But maybe
something is missing.

In our P entry in `MAINTAINERS` ("Subsystem Profile document") we ask
people to run them among other things, so I would perhaps suggest
having something like that too (or linking to ours if you prefer):

    https://rust-for-linux.com/contributing#submit-checklist-addendum

Of course new contributors will miss that initially, but actually
people find the doctests quite useful, so generally people get to run
them. At the end of the day, maintainers should test these too before
applying and otherwise someone will notice sooner or later.

I hope that helps!

Cheers,
Miguel

Re: [PATCH] rust: id_pool: fix example

[Alice Ryhl]

On Tue, Dec 02, 2025 at 12:13:33AM +0100, Miguel Ojeda wrote:
> On Mon, Dec 1, 2025 at 8:29 PM Yury Norov <yury.norov@gmail.com> wrote:
> >
> > Thanks Miguel,
> >
> > I applied this, but the fact that you've sent a second fix to
> > documentation that actually is a build fix, raises the questions.
> >
> > Because Rust documentation bears compilable chunks of code, I think
> > we need to enable rustdoc tests target by default, so that developers
> > will not send broken tests.
> 
> You're welcome!
> 
> This one is a Kconfig in the normal build, i.e.
> `CONFIG_RUST_KERNEL_DOCTESTS` (it is true that the other patch was for
> a different target).
> 
> If you mean enabling that by Kconfig default, then I don't think we
> can do that -- KUnit on its own (which this uses) is not meant for
> normal builds. Perhaps we could have something that just checks the
> build, but people should really run the doctests anyway, since they
> typically contain `assert!`s and so on that can be wrong.
> 
> If you mean enabling it in Intel's kernel test robot, then I think
> they do it (or at least I told them about it long ago). But maybe
> something is missing.
> 
> In our P entry in `MAINTAINERS` ("Subsystem Profile document") we ask
> people to run them among other things, so I would perhaps suggest
> having something like that too (or linking to ours if you prefer):
> 
>     https://rust-for-linux.com/contributing#submit-checklist-addendum
> 
> Of course new contributors will miss that initially, but actually
> people find the doctests quite useful, so generally people get to run
> them. At the end of the day, maintainers should test these too before
> applying and otherwise someone will notice sooner or later.

Yeah sorry I forgot to check the docs this time. I usually do run them,
but there are so many targets to check that sometimes I forget some of
them.

Alice

Re: [PATCH] rust: id_pool: fix example

[Yury Norov]

On Tue, Dec 02, 2025 at 12:22:52PM +0000, Alice Ryhl wrote:
> On Tue, Dec 02, 2025 at 12:13:33AM +0100, Miguel Ojeda wrote:
> > On Mon, Dec 1, 2025 at 8:29 PM Yury Norov <yury.norov@gmail.com> wrote:
> > >
> > > Thanks Miguel,
> > >
> > > I applied this, but the fact that you've sent a second fix to
> > > documentation that actually is a build fix, raises the questions.
> > >
> > > Because Rust documentation bears compilable chunks of code, I think
> > > we need to enable rustdoc tests target by default, so that developers
> > > will not send broken tests.
> > 
> > You're welcome!
> > 
> > This one is a Kconfig in the normal build, i.e.
> > `CONFIG_RUST_KERNEL_DOCTESTS` (it is true that the other patch was for
> > a different target).
> > 
> > If you mean enabling that by Kconfig default, then I don't think we
> > can do that -- KUnit on its own (which this uses) is not meant for
> > normal builds. Perhaps we could have something that just checks the
> > build, but people should really run the doctests anyway, since they
> > typically contain `assert!`s and so on that can be wrong.
> > 
> > If you mean enabling it in Intel's kernel test robot, then I think
> > they do it (or at least I told them about it long ago). But maybe
> > something is missing.
> > 
> > In our P entry in `MAINTAINERS` ("Subsystem Profile document") we ask
> > people to run them among other things, so I would perhaps suggest
> > having something like that too (or linking to ours if you prefer):
> > 
> >     https://rust-for-linux.com/contributing#submit-checklist-addendum
> > 
> > Of course new contributors will miss that initially, but actually
> > people find the doctests quite useful, so generally people get to run
> > them. At the end of the day, maintainers should test these too before
> > applying and otherwise someone will notice sooner or later.

No. Maintainers are not handy testers at your convenience.
 
> Yeah sorry I forgot to check the docs this time. I usually do run them,
> but there are so many targets to check that sometimes I forget some of
> them.

Don't be sorry, it's not your fault. The problem is that bearing
tests smeared among comments is a terrible idea, and now you reap
the benefits of someone else's bad design.

(Did I warn about it? Yes I did.)

The other problem is that Miguel replied in great details about how
well rust process is designed, and even attached an external link,
but didn't answer my question: how to enable rustdoc by default?

I followed this link by the way. It doesn't mention that rustdoc step
is mandatory. It doesn't provide steps for careful testing. It only
says:

        When submitting changes to Rust code documentation, please
        render them using the rustdoc target and ensure the result
        looks as expected. The Rust code documentation gets rendered
        at https://rust.docs.kernel.org.

Does it sound like:

        Rustdoc is a MANDATORY step! You MUST run rustdoc every single
        time, otherwise the build will get BROKEN. Even if you don't
        touch comments!

No, it does not. So, I wasted an hour just to find nothing. But there
are some good news: during that hour, I received an LKP email about
your error:

https://lore.kernel.org/oe-kbuild-all/202512020552.NejH5iaK-lkp@intel.com/

It's good that I started receiving such a traffic at all. But it's
especially good because I was on my way to submit a PR to Linus, and
that hour saved me a broken request. Andrew Morton on the previous
cycle wasn't that lucky, didn't he?

So, what's next. 

1. I will merge-fold Miguel's fixes into your patches and send my
   PR by the end of this week, not early the week as I usually do.
   Hopefully, no objections.

2. I will stop taking any rust patches unless the author explicitly
   mentions that he ran rustdoc on them.

3. I encourage rust community to spend this cycle on reviewing the
   development and submitting process. You guys decided to sprinkle
   compilable code across the comments, and you advocate it. Now
   please find a way for everyone to compile your comments (huh)
   during a routine development. And please make that knowledge
   very sound.

Thanks,
Yury

Re: [PATCH] rust: id_pool: fix example

[Miguel Ojeda]

On Tue, Dec 2, 2025 at 7:33 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> No. Maintainers are not handy testers at your convenience.

It definitely is your responsibility to test your tree. This is basic stuff.

And I am not sure what you are trying to imply here with your words in
your reply, but I personally find your message and its tone borderline
unacceptable. I was actually trying to help you, so it is doubly
annoying to get such a reply, to be honest.

I will reply to a few things below to keep the record straight now
that LPC is over and I had the chance to talk about this to a few
people.

> Don't be sorry, it's not your fault. The problem is that bearing
> tests smeared among comments is a terrible idea, and now you reap
> the benefits of someone else's bad design.
>
> (Did I warn about it? Yes I did.)

They are not comments: they are _documentation_, which is different,
not just conceptually, but also technically. From that idea, the rest
follows quite easily.

They are also not tests in the sense of "we write tests like this" --
they are primarily *examples*,  which are a fundamental part of the
documentation. They are not supposed to replace tests.

And it is important to understand that those examples would be there
*anyway* whether we compiled them or not. The fact that we actually
can not just build but runtime test them within KUnit is a very good
thing, because it ensures that the examples that developers wrote are
actually true *and remain true* even across refactoring/time.

That is, the examples will always reflect the reality of the API, the
asserts will always represent the true return values of the API, there
will be no typos of all kinds when typing the examples, and so on and
so forth.

Regardless of all that, other (non-Rust) maintainers do want to have doctests.

So, no, doctests are staying, and we are writing more of them, not
less. In fact, new APIs should come with examples unless there is a
good reason not to.

> The other problem is that Miguel replied in great details about how
> well rust process is designed,

The point was not to tell you about how "well designed" anything is
"in great detail", and I am not sure if you are sarcastically saying
this.

I simply gave you a link to our docs and suggested you could have a
similar field in the `MAINTAINERS` file, because in my experience
(which you can easily check in the mailing list) people actually do
write and test doctests.

As I said, I was just trying to help.

> and even attached an external link,
> but didn't answer my question: how to enable rustdoc by default?

I *did* answer your question, and pretty directly:

  1) I explained it was not a `rustdoc` target issue.

  2) I explained the Kconfig should not be enabled by default.

Now, it is clear that you are still quite confused about what failed
and that you didn't take the time to read into the link I provided, or
the docs, or the Kconfig option I mentioned.

So from your point of view, since you thought this was a different
topic, I guess you could have thought "Miguel is talking to me about
unrelated things". That is fine, but you can always say it politely,
and I would have tried to clear the confusion again.

> I followed this link by the way. It doesn't mention that rustdoc step
> is mandatory. It doesn't provide steps for careful testing.

It does (both) and I quote:

    When submitting changes to tests, including #[test]s, Kselftests
and examples inside Rust code documentation (i.e. "doctests", which
are transformed into KUnit tests), please test them.

You are just confusing the issue. This is about *testing*, not about
`rustdoc`, as I already explained in my previous message, so you read
the wrong bullet point.

It even clarifies what doctests are and even links to the "Testing" in
the kernel tree which is a page dedicated to explain this.

Even if you missed that, I even explicitly told you in my previous
message the Kconfig option you had to enable.

Even if the docs were wrong or may be improved, your message is just
not a productive way to engage.

> Does it sound like:
>
>         Rustdoc is a MANDATORY step! You MUST run rustdoc every single
>         time, otherwise the build will get BROKEN. Even if you don't
>         touch comments!

Yes, it does very explicitly explain the mandatory steps. No, they are
not comments. No, this is not about the `rustdoc` target. See above
for the quotes.

> No, it does not. So, I wasted an hour just to find nothing. But there

Yes, it does. And you didn't have to "find" anything. I *explicitly
told you* the Kconfig you needed in my previous message.

> It's good that I started receiving such a traffic at all. But it's
> especially good because I was on my way to submit a PR to Linus, and
> that hour saved me a broken request. Andrew Morton on the previous
> cycle wasn't that lucky, didn't he?

What do you mean? Where do you want to go with this? Please speak
clearly here -- veiled comments aren't useful nor polite.

> 2. I will stop taking any rust patches unless the author explicitly
>    mentions that he ran rustdoc on them.

Submitters are definitely supposed to test their code, just like for
any other code. And as a maintainer you are the one that decides what
is "good enough" etc.

But everyone misses things from time to time, sometimes authors are
newcomers, sometimes they may not have their full testing setup
nearby, etc., so you as a maintainer still need to test it anyway.

That is the point -- both submitters and maintainers are supposed to
work in tandem and double-check the other side, since anyone can miss
things.

To me, it sounds like just because you don't like doctests, you are
going to ask *everyone else* to do the testing for you, which
personally doesn't sound acceptable to me, to be honest.

I also find it a bit amusing, because you told me "Maintainers are not
handy testers at your convenience": To me, it sounds like *you* are
the one asking others to test your branch for you.

> 3. I encourage rust community to spend this cycle on reviewing the
>    development and submitting process. You guys decided to sprinkle
>    compilable code across the comments, and you advocate it. Now
>    please find a way for everyone to compile your comments (huh)
>    during a routine development. And please make that knowledge
>    very sound.

No. You don't get to blame others here. I am always happy to "review
the process" to improve it, but this issue has nothing to do with
that.

The process is quite clear, has been documented/followed/discussed for
a long time, and other subsystems and maintainers take advantage of
the feature and use it extensively.

It is obviously fine to not know about something (nobody can read
everything), but that doesn't mean you get to pass the buck and to
change how it all works just because you don't like doctests unless
you convince everyone using the feature that they shouldn't use it
anymore.

And it seems you also ignored from my message why the doctests are not
built in every config (which is very much done on purpose), and
instead keep pushing for it, which is not useful and is not going to
go anywhere.

Cheers,
Miguel