Project

General

Profile

Actions

Submit #2449

closed

INVARIANTS option fix.

Added by adamsaka about 12 years ago. Updated almost 12 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Start date:
11/06/2012
Due date:
% Done:

0%

Estimated time:

Description

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


Files

Actions #1

Updated by swildner about 12 years 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.
Actions #2

Updated by adamsaka about 12 years 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.

Actions #3

Updated by swildner about 12 years ago

OK, I've pushed that fix.

Actions #4

Updated by marino about 12 years ago

  • Status changed from New to Closed

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

Actions #5

Updated by vsrinivas almost 12 years 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;

Actions

Also available in: Atom PDF