Submit #2449

INVARIANTS option fix.

Added by adamsaka over 1 year ago. Updated over 1 year ago.

Status:ClosedStart date:11/06/2012
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

Description

With the INVARIANTS option defined the kernel does not build.
This small patch fixes the issue.

0001-Prevent-build-failure-when-INVARIANTS-option-disable.patch Magnifier - INVARIANTS option fix. (1.31 KB) adamsaka, 11/06/2012 06:20 AM

0001-Fix-INVARIANT-build-issue.patch Magnifier - New patch. (1.78 KB) adamsaka, 11/07/2012 09:21 PM

History

#1 Updated by swildner over 1 year ago

Here's some mental notes from when I looked into the issue some time ago:

* I think the "++spin->countb;" can be moved up into the INVARIANTS too. In fact, the whole "countb" member of struct spinlock seems to be INVARIANTS dependent (as in, it's checked only in INVARIANTS code, afaict).

* AFAIR, iwn(4) also has an INVARIANTS issue, which (afaict) isn't so easy to fix. If you want to look at it too, you might want to try to compile a LINT kernel without INVARIANTS.

#2 Updated by adamsaka over 1 year ago

> * I think the "++spin->countb;" can be moved up into the INVARIANTS too.

I agree. A new patch is attached with such changes. (Disregard the previous patch file)

countb is not referred to anywhere in the kernel code outside of Invariants code. I have left it's definition in the header files at this stage. It looks like some of the comments in those header files could use with an update. It appears the code increments or decrements counta. Rather than use a chase counter as the comments suggest.

>AFAIR, iwn(4) also has an INVARIANTS issue.

I'll have a look at a later stage.

#3 Updated by swildner over 1 year ago

OK, I've pushed that fix.

#4 Updated by marino over 1 year ago

  • Status changed from New to Closed

assumed fixed now. Reopen if assumption turns out to be bad.

#5 Updated by vsrinivas over 1 year ago

On Wed, Nov 07, 2012 at 09:21:41PM -0800, Adam Sakareassen via Redmine wrote:
>
>Issue #2449 has been updated by Adam Sakareassen.
>
>File 0001-Fix-INVARIANT-build-issue.patch added
>
>> * I think the "++spin->countb;" can be moved up into the INVARIANTS too.
>
>I agree. A new patch is attached with such changes. (Disregard the previous patch file)

In the version of the spinlock just before the s/x mode went in, the
++spin->countb was actually important for performance on a 48-core
(4 socket x 12-core) K10 machine I believe; there should be logs of
discussion on IRC from October or November 2011 discussing it.

>countb is not referred to anywhere in the kernel code outside of Invariants code. I have left it's definition in the header files at this stage. It looks like some of the comments in those header files could use with an update. It appears the code increments or decrements counta. Rather than use a chase counter as the comments suggest.
>

The chased counter comment was a relic from a past version of the spinlock code.

-- vs;

Also available in: Atom PDF