sys: add conf/std.debug, generic debugging options

[kp]

Summary

The new sys/conf/std.debug contains the list of debugging options
enabled by default in -CURRENT, so they don't need to be listed
individually in every kernel config.

Introduce *-DEBUG variants of the major kernel configs.

(cherry picked and modified from commit 4f8f9d708e6a4143f3b178bfab10d0a9b75ba2fe)

[olce]

Thanks!

sys/conf/std.debug

3

[markj]

This patch is against stable/14, not main

I very much want this (and for release media to start including a kernel.debug by default when possible, cc releng), but sys/conf/std.debug already exists on main. In particular, you've added a few extra _DEBUG options here which aren't in GENERIC. I think that's probably fine, but ideally we'd start by MFCing the commit which added std.debug.

[zlei]

I always have a GENERIC-DEBUG for stable branches, the content is similar. +1 for this.

[kp]

In D46871#1069247, @markj wrote:

This patch is against stable/14, not main

I very much want this (and for release media to start including a kernel.debug by default when possible, cc releng), but sys/conf/std.debug already exists on main. In particular, you've added a few extra _DEBUG options here which aren't in GENERIC. I think that's probably fine, but ideally we'd start by MFCing the commit which added std.debug.

I can (and should) start from that commit, but it changes the GENERIC configs, which isn't quite what we want on stable/14,stable/13.
I've done a cherry-pick along with major modifications and will update this review with that in a few minutes.

[kp]

Fix RISC-V comment.

[kp]

More comment fixes & remove unwanted changes in i386/MINIMAL

[markj]

Do these configs automatically get built with tinderbox and universe make targets? If so, this looks good to me aside from some nits I commented on inline.

sys/amd64/conf/GENERIC-DEBUG

4

The indentation here looks off.

11

Extra newline, here and in some of the other configs below.

sys/amd64/conf/NOTES

164

KCSAN appears twice.

sys/conf/std.debug

18

You might add commented out lines referencing the other subsystem debug options you had originally.

[bz]

sys/conf/std.debug

17

std.nodebug also has:

# Net80211 debugging
nooptions       IEEE80211_DEBUG

# USB debugging
nooptions       USB_DEBUG
nooptions       HID_DEBUG

# CAM debugging
nooptions       CAMDEBUG
nooptions       CAM_DEBUG_FLAGS

is there a reason to not add these as well (apart from CAM_DEBUG_FLAGS)? That said, I am not sure why this file shows up as new here?

[kp]

sys/conf/std.debug

17

That said, I am not sure why this file shows up as new here?

This is for stable/14 (and later I'll look at a variant for stable/13 too). std.debug already exists in main, but not on the stable branches.

[kp]

• fix whitespace
• add a few more (some commented out) debug options

I've also done a make tinderbox and confirmed that builds these new kernel configs.

[markj]

sys/conf/std.debug

22

KCOV and COVERAGE aren't really debug options, they're used for code coverage instrumentation. COVERAGE is the code coverage sanitizer, KCOV is a small device interface used by syzkaller to enable coverage collection from userspace.

I would just remove them from here.

33

It's not clear to me that we want these enabled by default. They aren't enabled in GENERIC on main.

[imp]

sys/conf/std.debug

33

Keep CAMDEBUG. It's fairly low overhead. Lose the FLAGS. It sets the default only and needs a value.

[kp]

Remove KCOV & CAM_DEBUG_FLAGS

[imp]

I'm curious why BUF_TRACKING is on this list... It changes the KBI so would create a bigger problem with not being able to load .ko's than it would solve. It's rarely useful to tracking down issues and introduces a decent amount of overhead to boot. So on the whole, I think we'd be better off without it, but maybe there's some good reason to include it.

sys/amd64/conf/NOTES

166

this is only tangentially related. It would be better as a separate commit imho.

sys/conf/std.debug

9

I'm not sure this is a good idea. I think it would slow things down too much. We don't normally enable it, and it's quite intrusive.
I'm not even sure BUF_TRACKING is a good idea. It changes the ABI in weird ways since it alters the layout of struct buf making it impossible to use packages that have .ko's that deal with buf.

[kp]

In D46871#1071719, @imp wrote:

I'm curious why BUF_TRACKING is on this list...

It's also in sys/conf/std.debug on main.

It changes the KBI so would create a bigger problem with not being able to load .ko's than it would solve.

We're going to have that problem anyway, between DEBUG and NODEBUG builds. I'm not sure why that's a concern.

It's rarely useful to tracking down issues and introduces a decent amount of overhead to boot. So on the w>hole, I think we'd be better off without it, but maybe there's some good reason to include it.

"It's enabled on main" is the extent of my reasoning. I don't particularly care about individual debug features, to be honest. I mostly only ever care about INVARIANTS and WITNESS, because those two are 95% of the value of a DEBUG build.
So I don't have particularly strong views here. I'm primarily interested in making it easier to build debug versions on stable branches, because it'll be more useful to build and test those than release versions.

[markj]

In D46871#1071775, @kp wrote:

In D46871#1071719, @imp wrote:

I'm curious why BUF_TRACKING is on this list...

It's also in sys/conf/std.debug on main.

It changes the KBI so would create a bigger problem with not being able to load .ko's than it would solve.

We're going to have that problem anyway, between DEBUG and NODEBUG builds. I'm not sure why that's a concern.

It's rarely useful to tracking down issues and introduces a decent amount of overhead to boot. So on the w>hole, I think we'd be better off without it, but maybe there's some good reason to include it.

"It's enabled on main" is the extent of my reasoning. I don't particularly care about individual debug features, to be honest. I mostly only ever care about INVARIANTS and WITNESS, because those two are 95% of the value of a DEBUG build.
So I don't have particularly strong views here. I'm primarily interested in making it easier to build debug versions on stable branches, because it'll be more useful to build and test those than release versions.

The KBI concerns aren't very relevant in practice:

• most fields in struct buf are not affected, except the page array, and most consumers have no business poking around there,
• the size of struct buf is already dynamic, so consumers can't deal with arrays of them,
• buffer cache consumers never allocate bufs, that's handled by vfs_bio.c,
• there aren't many (any?) 3rd party modules that people care about that manipulate bufs.

[imp]

In D46871#1071808, @markj wrote:

In D46871#1071775, @kp wrote:

In D46871#1071719, @imp wrote:

I'm curious why BUF_TRACKING is on this list...

It's also in sys/conf/std.debug on main.

It changes the KBI so would create a bigger problem with not being able to load .ko's than it would solve.

We're going to have that problem anyway, between DEBUG and NODEBUG builds. I'm not sure why that's a concern.

It's rarely useful to tracking down issues and introduces a decent amount of overhead to boot. So on the w>hole, I think we'd be better off without it, but maybe there's some good reason to include it.

"It's enabled on main" is the extent of my reasoning. I don't particularly care about individual debug features, to be honest. I mostly only ever care about INVARIANTS and WITNESS, because those two are 95% of the value of a DEBUG build.
So I don't have particularly strong views here. I'm primarily interested in making it easier to build debug versions on stable branches, because it'll be more useful to build and test those than release versions.

The KBI concerns aren't very relevant in practice:

• most fields in struct buf are not affected, except the page array, and most consumers have no business poking around there,
• the size of struct buf is already dynamic, so consumers can't deal with arrays of them,
• buffer cache consumers never allocate bufs, that's handled by vfs_bio.c,
• there aren't many (any?) 3rd party modules that people care about that manipulate bufs.

Hmmm... There are several fs modules that we care about. However, I don't think any of them manipulate b_pages, which is the only visible thing normally... and I don't think that there's any that hook into that #ifdef. So I'll withdraw my objection. I do think that it is a bit too high overhead, but I'll take a wait-and-see approach then.