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

Support generating all docs, introduce doc_cfg and doc_all #446

Merged
merged 2 commits into from
Jul 15, 2021

Conversation

ojeda
Copy link
Member

@ojeda ojeda commented Jul 15, 2021

See individual commits for details.

See #442 for more details.

@nbdd0121
Copy link
Member

Any reason not to enable all docs?

@ojeda
Copy link
Member Author

ojeda commented Jul 15, 2021

What do you mean?

@nbdd0121
Copy link
Member

I think we should enable all docs for rustdoc target and not have rustdoc-all.

@ojeda
Copy link
Member Author

ojeda commented Jul 15, 2021

I thought about that too (to simplify), but on the other hand, supporting custom docs is very easy, so I thought it was nice to keep it.

e.g. imagine a Linux distribution packaging the docs for a given kernel they provide.

We could also do further things for the custom ones, like a smaller banner when customized for a particular config (including a link to it, etc.).

@ojeda
Copy link
Member Author

ojeda commented Jul 15, 2021

I mean, I am not saying it will be used a lot, but on the other hand, people may actually use it if we have it (i.e. the kind of thing nobody will ask about if it is not there, but may end up being used if it is there).

@ojeda
Copy link
Member Author

ojeda commented Jul 15, 2021

Another use case is generating the docs faster when you are developing (because typically one only cares about the current .config, e.g. when working in a particular subsystem); while rustdoc-all is only intended for the bot that will generate the ones for kernel.org which would take more time.

@nbdd0121
Copy link
Member

Or it could be argued that people may use it because it is there, while they shouldn't be using it and should use rustdoc-all instead.

I would say documenting all items should be the default and would better occupy the name rustdoc, and if people want a trimmed down version it should be a different target.

Another use case is generating the docs faster when you are developing (because typically one only cares about the current .config); while rustdoc-all is only intended for the bot that will generate the ones for kernel.org which would take more time.

Well, that's not really a convincing argument. Kernel crate docs would only be regenerated if kernel crate is changed, so for driver devs wouldn't trigger a re-generation for the docs. Doc generation also does not require borrow checking or code generation, so it is much faster than compilation.

Devs should be able to look into all documentations rather than just the ones being used; e.g. they may forget to add a few CONFIG dependencies and then wouldn't notice that some Rust binding exists.

@ojeda
Copy link
Member Author

ojeda commented Jul 15, 2021

I would say documenting all items should be the default and would better occupy the name rustdoc, and if people want a trimmed down version it should be a different target.

As a subsystem developer, if I am generating the docs to check my changes, then I prefer the trimmed down version; i.e. there is not much use about many other subsystems (including things from other architectures) that would be around otherwise.

Basically, I see two use cases:

  • I want to explore the docs, i.e. I want to see everything: I go to kernel.org (or generate them myself).
  • I want to check my changes, i.e. I only need to see what I changed: I generate them locally because nobody has them pregenerated anyway.

Doc generation also does not require borrow checking or code generation, so it is much faster than compilation.

Yes, but if Rust grows in the kernel, we will have a lot of things. Being able to skip entire subsystems and architectures can make quite a difference. It is like building allmodconfig vs. your particular config.

they may forget to add a few CONFIG dependencies and then wouldn't notice that some Rust binding exists.

This is fair, it could potentially cause some confusion -- perhaps not in the case you mention (i.e. a driver developer knows the bindings/configs they need and they will notice otherwise), but for things like optional functionality (e.g. utility functions that for some reason are gated) that they may not notice if they rely on their own docs for exploration. Hmm...

@ojeda
Copy link
Member Author

ojeda commented Jul 15, 2021

It is not particularly important anyway, so we can always prioritize simplicity for the moment.

ojeda added 2 commits July 15, 2021 23:30
Closes Rust-for-Linux#442.

Suggested-by: Finn Behrens <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
As an example, use `doc(cfg(...))` and `cfg(doc)` to gate `sysctl`.

Signed-off-by: Miguel Ojeda <[email protected]>
@ojeda
Copy link
Member Author

ojeda commented Jul 15, 2021

Let's go with the simplest way. I have kept this in an issue in case someone needs the feature in the future: #447

@ojeda ojeda merged commit bc33ea8 into Rust-for-Linux:rust Jul 15, 2021
@ojeda ojeda deleted the doc_cfg branch July 15, 2021 22:57
ojeda pushed a commit that referenced this pull request Nov 29, 2021
When we try to add an IPv6 nexthop and IPv6 is not enabled
(!CONFIG_IPV6) we'll hit a NULL pointer dereference[1] in the error path
of nh_create_ipv6() due to calling ipv6_stub->fib6_nh_release. The bug
has been present since the beginning of IPv6 nexthop gateway support.
Commit 1aefd3d ("ipv6: Add fib6_nh_init and release to stubs") tells
us that only fib6_nh_init has a dummy stub because fib6_nh_release should
not be called if fib6_nh_init returns an error, but the commit below added
a call to ipv6_stub->fib6_nh_release in its error path. To fix it return
the dummy stub's -EAFNOSUPPORT error directly without calling
ipv6_stub->fib6_nh_release in nh_create_ipv6()'s error path.

[1]
 Output is a bit truncated, but it clearly shows the error.
 BUG: kernel NULL pointer dereference, address: 000000000000000000
 #PF: supervisor instruction fetch in kernel modede
 #PF: error_code(0x0010) - not-present pagege
 PGD 0 P4D 0
 Oops: 0010 [#1] PREEMPT SMP NOPTI
 CPU: 4 PID: 638 Comm: ip Kdump: loaded Not tainted 5.16.0-rc1+ #446
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-4.fc34 04/01/2014
 RIP: 0010:0x0
 Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
 RSP: 0018:ffff888109f5b8f0 EFLAGS: 00010286^Ac
 RAX: 0000000000000000 RBX: ffff888109f5ba28 RCX: 0000000000000000
 RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8881008a2860
 RBP: ffff888109f5b9d8 R08: 0000000000000000 R09: 0000000000000000
 R10: ffff888109f5b978 R11: ffff888109f5b948 R12: 00000000ffffff9f
 R13: ffff8881008a2a80 R14: ffff8881008a2860 R15: ffff8881008a2840
 FS:  00007f98de70f100(0000) GS:ffff88822bf00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: ffffffffffffffd6 CR3: 0000000100efc000 CR4: 00000000000006e0
 Call Trace:
  <TASK>
  nh_create_ipv6+0xed/0x10c
  rtm_new_nexthop+0x6d7/0x13f3
  ? check_preemption_disabled+0x3d/0xf2
  ? lock_is_held_type+0xbe/0xfd
  rtnetlink_rcv_msg+0x23f/0x26a
  ? check_preemption_disabled+0x3d/0xf2
  ? rtnl_calcit.isra.0+0x147/0x147
  netlink_rcv_skb+0x61/0xb2
  netlink_unicast+0x100/0x187
  netlink_sendmsg+0x37f/0x3a0
  ? netlink_unicast+0x187/0x187
  sock_sendmsg_nosec+0x67/0x9b
  ____sys_sendmsg+0x19d/0x1f9
  ? copy_msghdr_from_user+0x4c/0x5e
  ? rcu_read_lock_any_held+0x2a/0x78
  ___sys_sendmsg+0x6c/0x8c
  ? asm_sysvec_apic_timer_interrupt+0x12/0x20
  ? lockdep_hardirqs_on+0xd9/0x102
  ? sockfd_lookup_light+0x69/0x99
  __sys_sendmsg+0x50/0x6e
  do_syscall_64+0xcb/0xf2
  entry_SYSCALL_64_after_hwframe+0x44/0xae
 RIP: 0033:0x7f98dea28914
 Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 80 00 00 00 00 48 8d 05 e9 5d 0c 00 8b 00 85 c0 75 13 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 41 89 d4 55 48 89 f5 53
 RSP: 002b:00007fff859f5e68 EFLAGS: 00000246 ORIG_RAX: 000000000000002e2e
 RAX: ffffffffffffffda RBX: 00000000619cb810 RCX: 00007f98dea28914
 RDX: 0000000000000000 RSI: 00007fff859f5ed0 RDI: 0000000000000003
 RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000008
 R10: fffffffffffffce6 R11: 0000000000000246 R12: 0000000000000001
 R13: 000055c0097ae520 R14: 000055c0097957fd R15: 00007fff859f63a0
 </TASK>
 Modules linked in: bridge stp llc bonding virtio_net

Cc: [email protected]
Fixes: 53010f9 ("nexthop: Add support for IPv6 gateways")
Signed-off-by: Nikolay Aleksandrov <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants