Add CTASSERTs for where CTLFLAG_MPSAFE is redundant.

[mjoras]

Summary

This also fixes up the build breaks that result from this change. Most
of the sysctl(9) macros besides _PROC are MPSAFE by default, so we might
as well catch pointless uses of the flag.

Test Plan

make tinderbox

[rpokala]

Looks fine to my relatively-untrained eye.

[jtl]

I agree with eliminating the pointless uses of the flag; however, I don't understand why we need a CTASSERT to catch this. It doesn't break anything. At most, it seems like it is simply a style violation. And, I don't think CTASSERTs are necessary to enforce style.

If you think there is a really good reason to add CTASSERTs, please update the man page to indicate this. At least, that will document it for developers (like me :-) ) who regularly add them out of habit.

[mjoras]

In D14604#306835, @jtl wrote:

I agree with eliminating the pointless uses of the flag; however, I don't understand why we need a CTASSERT to catch this. It doesn't break anything. At most, it seems like it is simply a style violation. And, I don't think CTASSERTs are necessary to enforce style.

If you think there is a really good reason to add CTASSERTs, please update the man page to indicate this. At least, that will document it for developers (like me :-) ) who regularly add them out of habit.

imp actually just added stating not to use it to the man page in r330371, is that good enough? :) I agree that it is basically a style problem, but I don't know how else we are supposed to catch these other than periodically grepping the tree.

[jtl]

In D14604#306967, @mjoras wrote:

imp actually just added stating not to use it to the man page in r330371, is that good enough? :) I agree that it is basically a style problem, but I don't know how else we are supposed to catch these other than periodically grepping the tree.

IMHO, the man page should also state that there is a CTASSERT for it. And, the man page should also explain why this is serious enough that we are going to CTASSERT on style violations.

Ideally, this would be in some sort of linter that we encourage people to run before they commit. Or, you can make the CTASSERT only be active for LINT builds. Or, we can deal with it the same way we deal with most other style violations: peer pressure by emailing the offender after a commit.

A CTASSERT just seems like too heavy of a hammer for this. But, maybe I'm missing the importance?

[mjoras]

In D14604#307002, @jtl wrote:

In D14604#306967, @mjoras wrote:

imp actually just added stating not to use it to the man page in r330371, is that good enough? :) I agree that it is basically a style problem, but I don't know how else we are supposed to catch these other than periodically grepping the tree.

IMHO, the man page should also state that there is a CTASSERT for it. And, the man page should also explain why this is serious enough that we are going to CTASSERT on style violations.

Ideally, this would be in some sort of linter that we encourage people to run before they commit. Or, you can make the CTASSERT only be active for LINT builds. Or, we can deal with it the same way we deal with most other style violations: peer pressure by emailing the offender after a commit.

A CTASSERT just seems like too heavy of a hammer for this. But, maybe I'm missing the importance?

Yeah, I see your point. I guess I don't view a CTASSERT as catastrophic as that. Generally people use it for things that are very "big hammery" (e.g. the size of this on-wire struct must be exactly 14 bytes), but breaking the build itself doesn't seem like that big of a deal?

The point about putting it in the LINT build is an interestng one. Would you oppose using CTASSERTs in that context? I'd much prefer to just plug this into some pre-existing linter, but we unfortunately lack such a capability :(.