Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc_cfg #442

Closed
kloenk opened this issue Jul 12, 2021 · 5 comments
Closed

doc_cfg #442

kloenk opened this issue Jul 12, 2021 · 5 comments
Labels
• docs Related to `Documentation/rust/`, `samples/`, generated docs, doctests, typos...

Comments

@kloenk
Copy link
Member

kloenk commented Jul 12, 2021

I would like to use doc_cfg for documentation. I don't have a clue about the current build system. Would it be trivial to allow? @ojeda can you do that? Also, is this allowed, or would it be a to unstable feature?

@kloenk kloenk added the • docs Related to `Documentation/rust/`, `samples/`, generated docs, doctests, typos... label Jul 12, 2021
@kloenk
Copy link
Member Author

kloenk commented Jul 12, 2021

I figured out that I can just enable it (in #439). But We should discuss if I'm allowed to use it :-)

@ojeda
Copy link
Member

ojeda commented Jul 14, 2021

Not sure what you mean by allowing it in the current build system -- all unstable features are allowed inside rust/, so you should not need any change in particular.

The big question is, as usual, whether to add it to the list of unstable features or not. While for "custom" docs (i.e. docs generated for a given .config) it is fine to skip things that are not enabled, for the "global" docs, i.e. those that we will put in kernel.org, it would definitely nice to document everything. And I assume the "global" docs are the ones that most people will read.

From a quick look, there is still discussion going around doc_cfg, including talk about automatically inferring the doc(cfg(...)) attributes from the usual ones (to avoid duplication) etc., so there are things that may change and may take a while to get stabilized. On the other hand, it seems the feature will get stabilized one way or the other.

Ideally the compiler would ignore this feature when not generating the docs (so that it would not "count" as an unstable feature when not generating docs).

I have taken a look at the generated docs for something like:

#[cfg(any(CONFIG_SYSCTL, doc))]
#[doc(cfg(CONFIG_SYSCTL))]
pub mod sysctl;

and it looks very nice!

@ojeda
Copy link
Member

ojeda commented Jul 14, 2021

Actually, that gives me an idea: rustdoc could have a way to provide it with a custom map with the "nicer names" (like it has, hardcoded, for some key-values e.g. unix -> "Unix").

On our side, we could then automatically fill it with the titles from Kconfig, or something like that. For instance, for CONFIG_NET we could have the banner with:

This is supported when Networking (CONFIG_NET) is enabled only.

instead of:

This is supported on CONFIG_NET only.

(It looks worse here than in the generated docs).

@nbdd0121
Copy link
Member

The big question is, as usual, whether to add it to the list of unstable features or not.

Given that we pin to a specify Rust version, I think we could be more liberal when it comes to use unstable features that wouldn't be difficult to remove in the future. For things like doc_cfg even it's gone from rustc, we can remove all usages easily, so I don't think it'll be problematic to use it.

Actually, that gives me an idea: rustdoc could have a way to provide it with a custom map with the "nicer names" (like it has, hardcoded, for some key-values e.g. unix -> "Unix").

On our side, we could then automatically fill it with the titles from Kconfig, or something like that. For instance, for CONFIG_NET we could have the banner with:

Supported only when Networking support (CONFIG_NET) is enabled.

instead of:

This is supported on CONFIG_NET only.

Sounds like something doable by some simple text postprocessing :)

@ojeda
Copy link
Member

ojeda commented Jul 14, 2021

Given that we pin to a specify Rust version, I think we could be more liberal when it comes to use unstable features that wouldn't be difficult to remove in the future. For things like doc_cfg even it's gone from rustc, we can remove all usages easily, so I don't think it'll be problematic to use it.

The problem is not so much about removing them (which introduces churn, specially in mainline, but it is not a big deal), but about getting sooner to the point where we can compile the kernel without unstable features. In general, we should see the pinning as temporary, and work towards avoiding it, rather than the opposite (i.e. adding more features :)

Sounds like something doable by some simple text postprocessing :)

Definitely! I usually try to describe things in terms of what upstream could provide as a feature.

Submitted, by the way: rust-lang/rust#87139.

ojeda added a commit to ojeda/linux that referenced this issue Jul 14, 2021
Closes Rust-for-Linux#442.

Suggested-by: Finn Behrens <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
@ojeda ojeda closed this as completed in faf7ea6 Jul 15, 2021
ojeda pushed a commit that referenced this issue Feb 22, 2025
commit 7378aeb upstream.

In the rproc_alloc() function, on error, put_device(&rproc->dev) is
called, leading to the call of the rproc_type_release() function.
An error can occurs before ida_alloc is called.

In such case in rproc_type_release(), the condition (rproc->index >= 0) is
true as rproc->index has been  initialized to 0.
ida_free() is called reporting a warning:
[    4.181906] WARNING: CPU: 1 PID: 24 at lib/idr.c:525 ida_free+0x100/0x164
[    4.186378] stm32-display-dsi 5a000000.dsi: Fixed dependency cycle(s) with /soc/dsi@5a000000/panel@0
[    4.188854] ida_free called for id=0 which is not allocated.
[    4.198256] mipi-dsi 5a000000.dsi.0: Fixed dependency cycle(s) with /soc/dsi@5a000000
[    4.203556] Modules linked in: panel_orisetech_otm8009a dw_mipi_dsi_stm(+) gpu_sched dw_mipi_dsi stm32_rproc stm32_crc32 stm32_ipcc(+) optee(+)
[    4.224307] CPU: 1 UID: 0 PID: 24 Comm: kworker/u10:0 Not tainted 6.12.0 #442
[    4.231481] Hardware name: STM32 (Device Tree Support)
[    4.236627] Workqueue: events_unbound deferred_probe_work_func
[    4.242504] Call trace:
[    4.242522]  unwind_backtrace from show_stack+0x10/0x14
[    4.250218]  show_stack from dump_stack_lvl+0x50/0x64
[    4.255274]  dump_stack_lvl from __warn+0x80/0x12c
[    4.260134]  __warn from warn_slowpath_fmt+0x114/0x188
[    4.265199]  warn_slowpath_fmt from ida_free+0x100/0x164
[    4.270565]  ida_free from rproc_type_release+0x38/0x60
[    4.275832]  rproc_type_release from device_release+0x30/0xa0
[    4.281601]  device_release from kobject_put+0xc4/0x294
[    4.286762]  kobject_put from rproc_alloc.part.0+0x208/0x28c
[    4.292430]  rproc_alloc.part.0 from devm_rproc_alloc+0x80/0xc4
[    4.298393]  devm_rproc_alloc from stm32_rproc_probe+0xd0/0x844 [stm32_rproc]
[    4.305575]  stm32_rproc_probe [stm32_rproc] from platform_probe+0x5c/0xbc

Calling ida_alloc earlier in rproc_alloc ensures that the rproc->index is
properly set.

Fixes: 08333b9 ("remoteproc: Directly use ida_alloc()/free()")
Signed-off-by: Arnaud Pouliquen <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mathieu Poirier <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
• docs Related to `Documentation/rust/`, `samples/`, generated docs, doctests, typos...
Development

No branches or pull requests

3 participants