Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

cpu_intr_p() does not exist for all ports

5 views
Skip to first unread message

Paul Goyette

unread,
Dec 5, 2007, 8:41:24 PM12/5/07
to
For the last two days, a full release cannot be built for amd64 or i386
because of compile failure for sys/kern/kern_rwlock.c

It seems that the XEN_* kernels want to #define LOCKDEBUG which in turn
causes RW_ASSERT macro to actually do something. At line 257 in the
above file we attempt to call cpu_intr_p() which doesn't exist for all
ports; it is indeed missing in sources which were updated via CVS just
a few minutes ago.

The only other place where cpu_intr_p() is called is in subr_lockdebug.c
but that code is currently disabled via a #ifdef notyet.

I propose the following patch should be applied until all ports get an
implementation of cpu_intr_p() in order to allow kernels with LOCKDEBUG
to once again compile.

Index: kern_rwlock.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.13
diff -u -p -r1.13 kern_rwlock.c
--- kern_rwlock.c 4 Dec 2007 09:13:59 -0000 1.13
+++ kern_rwlock.c 6 Dec 2007 01:35:52 -0000
@@ -254,7 +254,9 @@ rw_vector_enter(krwlock_t *rw, const krw
l = curlwp;
curthread = (uintptr_t)l;

+#ifdef notyet /* Disable until cpu_intr_p() exists for all ports */
RW_ASSERT(rw, !cpu_intr_p());
+#endif # notyet
RW_ASSERT(rw, curthread != 0);
RW_WANTLOCK(rw, op);


----------------------------------------------------------------------
| Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | pa...@whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoy...@juniper.net |
----------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-...@muc.de

Michael Lorenz

unread,
Dec 5, 2007, 10:07:45 PM12/5/07
to
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

On Dec 5, 2007, at 20:41, Paul Goyette wrote:

> For the last two days, a full release cannot be built for amd64 or
> i386 because of compile failure for sys/kern/kern_rwlock.c
>
> It seems that the XEN_* kernels want to #define LOCKDEBUG which in
> turn causes RW_ASSERT macro to actually do something. At line 257
> in the above file we attempt to call cpu_intr_p() which doesn't
> exist for all ports; it is indeed missing in sources which were
> updated via CVS just a few minutes ago.

For the records - I just ran into the same problem on sparc and sparc64.

have fun
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)

iQEVAwUBR1dngspnzkX8Yg2nAQJHKwf+JbUD6mfLyhG/BZgYUCSI6EBzmzvCwx4L
b2JwrkciTo5SGiQ9vUALg1by3GVZZFe62TMap3ijO7/Wr2Xj8pm4ymsnzv43R8Kk
R0MPk917N5L0IzhuFPOFba0vspNs0z/LDCTOxrfBqq3qO8mIU21OW4eATZ6OGJCP
ohx5K0SQrn1KVoBlnAmhqR9xgtl/FjoQajqpLjgJQdE5pbhOV+wXA8AgydUQBWlT
4b9Ag9hzqYu8162SPEbQtrOw0JWuTUSBwbTTmtIhWUnjysgx7ERphtl4Pkkxnfq/
rdI7ocMNSfHUrUWe+hEe+f1uHV4c5wHsYDS4HyEYfkxw+ejexr64bQ==
=4OWa
-----END PGP SIGNATURE-----

Izumi Tsutsui

unread,
Dec 6, 2007, 7:18:47 AM12/6/07
to
maca...@NetBSD.org wrote:

> > For the last two days, a full release cannot be built for amd64 or
> > i386 because of compile failure for sys/kern/kern_rwlock.c
> >
> > It seems that the XEN_* kernels want to #define LOCKDEBUG which in
> > turn causes RW_ASSERT macro to actually do something. At line 257
> > in the above file we attempt to call cpu_intr_p() which doesn't
> > exist for all ports; it is indeed missing in sources which were
> > updated via CVS just a few minutes ago.
>
> For the records - I just ran into the same problem on sparc and sparc64.

Why don't you implement MD cpu_intr_p() functions by yourself?
(I think Andrew should focus on his own work..)

Even a dummy cpu_intr_p() (which returns always false) is better
than disabling the assertion among all ports.
---
Izumi Tsutsui

Paul Goyette

unread,
Dec 6, 2007, 7:53:45 AM12/6/07
to
On Thu, 6 Dec 2007, Izumi Tsutsui wrote:

> maca...@NetBSD.org wrote:
>
> Why don't you implement MD cpu_intr_p() functions by yourself?
> (I think Andrew should focus on his own work..)
>
> Even a dummy cpu_intr_p() (which returns always false) is better
> than disabling the assertion among all ports.

I didn't implement a real one because I don't know enough about what
it's supposed to be doing. I hadn't thought of doing a dummy one.

It seems that even on those ports where the function is defined, it's
not consistently located. It seems to show up in several different
source files on different ports. Which file is best?

But isn't this a _part_ of Andy's work? He's the one who checked in
this change a couple days ago?

I'd be happy to write a dummy function for the ports that I use, but I
haven't a clue how to write a dummy machine-independant version and have
the MD version supersede it where the MD version exists. It doesn't
seem smart to implement a separate dummy function for each port.

----------------------------------------------------------------------
| Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | pa...@whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoy...@juniper.net |
----------------------------------------------------------------------

--

Izumi Tsutsui

unread,
Dec 6, 2007, 8:14:23 AM12/6/07
to
pa...@whooppee.com wrote:

> It seems that even on those ports where the function is defined, it's
> not consistently located. It seems to show up in several different
> source files on different ports. Which file is best?

It seems to indicate if the kernel is in interrupt or not,
so sources which include interrupt handler may be appropriate.
(sys/arch/xen/x86/intr.c for xen?)

> But isn't this a _part_ of Andy's work? He's the one who checked in
> this change a couple days ago?

I think he should focus on works which can be handled only by him.
We should support him by other works we can do.

> I'd be happy to write a dummy function for the ports that I use, but I
> haven't a clue how to write a dummy machine-independant version and have
> the MD version supersede it where the MD version exists. It doesn't
> seem smart to implement a separate dummy function for each port.

Maybe an easy way is to put the dummy function in kern_rwlock.c
itself and use __weak_alias()?
---
Izumi Tsutsui

Michael Lorenz

unread,
Dec 6, 2007, 8:39:31 AM12/6/07
to
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

On Dec 6, 2007, at 07:18, Izumi Tsutsui wrote:

> maca...@NetBSD.org wrote:
>
>>> For the last two days, a full release cannot be built for amd64 or
>>> i386 because of compile failure for sys/kern/kern_rwlock.c
>>>
>>> It seems that the XEN_* kernels want to #define LOCKDEBUG which in
>>> turn causes RW_ASSERT macro to actually do something. At line 257
>>> in the above file we attempt to call cpu_intr_p() which doesn't
>>> exist for all ports; it is indeed missing in sources which were
>>> updated via CVS just a few minutes ago.
>>
>> For the records - I just ran into the same problem on sparc and
>> sparc64.
>
> Why don't you implement MD cpu_intr_p() functions by yourself?
> (I think Andrew should focus on his own work..)

If it's trivial enough I will. I had other things to do so I didn't
investigate further.

> Even a dummy cpu_intr_p() (which returns always false) is better
> than disabling the assertion among all ports.

Ok.

have fun
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)

iQEVAwUBR1f7lMpnzkX8Yg2nAQKhYwgAqLXfypWgqzlDbDs/ftTRz3ObvJ7wyZaJ
xP7Z07AO22EslnWdZCaK1P8ckzo6SUbKxaHs7aY7t2cufbO2H48lGfwhQHq3pMWo
CabiVrxZix+yUruZH1ShS3l78p+miX6lBneSb3D0/E6K21ZsGht0Dr3EbMTyrM7Q
FAYM87U2rr/7Dc2x7T7Iw0GynIaAUThCUQX7lsxZwBZCExIXrUolluNntEDI6DMq
MrYI15xqJet+zVszT8ZDz9Vte8AqaoDqA5qZnSqwaxKVP2ggwmH+YW7C+OjDwkh1
2EaK2xa5wgBo7VOrtq/o+Js0eP/lGyK3UfHZ8mvc09ocYm1BVKORLA==
=2OES
-----END PGP SIGNATURE-----

Paul Goyette

unread,
Dec 6, 2007, 8:21:33 AM12/6/07
to
On Thu, 6 Dec 2007, Izumi Tsutsui wrote:

> pa...@whooppee.com wrote:
>
>> It seems that even on those ports where the function is defined, it's
>> not consistently located. It seems to show up in several different
>> source files on different ports. Which file is best?
>
> It seems to indicate if the kernel is in interrupt or not,
> so sources which include interrupt handler may be appropriate.
> (sys/arch/xen/x86/intr.c for xen?)

OK. BTW, even on those ports which _do_ have a cpu_intr_p() function,
it doesn't seem to be exported in any header files. So I'm not sure how
we could ever successfully compile any kernel with LOCKDEBUG turned on.

:)

>> But isn't this a _part_ of Andy's work? He's the one who checked in
>> this change a couple days ago?
>
> I think he should focus on works which can be handled only by him.
> We should support him by other works we can do.

OK, I guess that's reasonable.

>> I'd be happy to write a dummy function for the ports that I use, but I
>> haven't a clue how to write a dummy machine-independant version and have
>> the MD version supersede it where the MD version exists. It doesn't
>> seem smart to implement a separate dummy function for each port.
>
> Maybe an easy way is to put the dummy function in kern_rwlock.c
> itself and use __weak_alias()?

Oh, good. Now I get to figure out what a __weak_alias() is and how to
use it!

----------------------------------------------------------------------
| Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | pa...@whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoy...@juniper.net |
----------------------------------------------------------------------

--

Izumi Tsutsui

unread,
Dec 6, 2007, 9:01:29 AM12/6/07
to
pa...@whooppee.com wrote:

> OK. BTW, even on those ports which _do_ have a cpu_intr_p() function,
> it doesn't seem to be exported in any header files.

It's declared in <sys/cpu.h>.
---
Izumi Tsutsui

Paul Goyette

unread,
Dec 6, 2007, 8:46:18 AM12/6/07
to
On Thu, 6 Dec 2007, Michael Lorenz wrote:

>> Why don't you implement MD cpu_intr_p() functions by yourself?
>> (I think Andrew should focus on his own work..)
>

> If it's trivial enough I will. I had other things to do so I didn't
> investigate further.
>

>> Even a dummy cpu_intr_p() (which returns always false) is better
>> than disabling the assertion among all ports.

I'm compiling with a dummy routine now, using weak_alias as was
suggested.


----------------------------------------------------------------------
| Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | pa...@whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoy...@juniper.net |
----------------------------------------------------------------------

--

Paul Goyette

unread,
Dec 6, 2007, 9:05:50 AM12/6/07
to
On Thu, 6 Dec 2007, Izumi Tsutsui wrote:

>> I'd be happy to write a dummy function for the ports that I use, but I
>> haven't a clue how to write a dummy machine-independant version and have
>> the MD version supersede it where the MD version exists. It doesn't
>> seem smart to implement a separate dummy function for each port.
>
> Maybe an easy way is to put the dummy function in kern_rwlock.c
> itself and use __weak_alias()?

OK, I've successfully built both i386 and amd64 'build.sh release' with
the following patch.

I noticed that Andy just checked in a fix for x86 (includes both amd64
and i386), so perhaps this isn't needed after all.

Index: kern_rwlock.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.13
diff -u -p -r1.13 kern_rwlock.c
--- kern_rwlock.c 4 Dec 2007 09:13:59 -0000 1.13

+++ kern_rwlock.c 6 Dec 2007 13:57:23 -0000
@@ -110,6 +110,20 @@ do { \

#endif /* DIAGNOSTIC */

+/* XXX
+ * XXX For platforms that don't implement a way to tell if we're running
+ * XXX in interrupt context, just pretend that we're never interrupted.
+ * XXX
+ */
+bool __cpu_intr_p(void);
+
+bool
+__cpu_intr_p(void)
+{
+ return FALSE;
+}
+__weak_alias(cpu_intr_p, __cpu_intr_p)
+
/*
* For platforms that use 'simple' RW locks.
*/


----------------------------------------------------------------------
| Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | pa...@whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoy...@juniper.net |
----------------------------------------------------------------------

--

Andrew Doran

unread,
Dec 6, 2007, 9:24:53 AM12/6/07
to
On Wed, Dec 05, 2007 at 05:41:24PM -0800, Paul Goyette wrote:

> For the last two days, a full release cannot be built for amd64 or i386
> because of compile failure for sys/kern/kern_rwlock.c

..


> I propose the following patch should be applied until all ports get an
> implementation of cpu_intr_p() in order to allow kernels with LOCKDEBUG
> to once again compile.

It would break again the next time it is enabled. It also goes a bit beyond
the number of ports, because some ports have multiple flavours of interrupt
dispatch code. I have added cpu_intr_p() for xen so it should be fixed now.

Thanks,
Andrew

Paul Goyette

unread,
Dec 6, 2007, 9:28:49 AM12/6/07
to
On Thu, 6 Dec 2007, Andrew Doran wrote:

> It would break again the next time it is enabled. It also goes a bit beyond
> the number of ports, because some ports have multiple flavours of interrupt
> dispatch code. I have added cpu_intr_p() for xen so it should be fixed now.

Yes, I've just compiled both amd64 and i386 successfully with your
latest fix. Thanks.

Martin Husemann

unread,
Dec 6, 2007, 9:31:04 AM12/6/07
to
On Thu, Dec 06, 2007 at 02:24:53PM +0000, Andrew Doran wrote:
> It would break again the next time it is enabled. It also goes a bit beyond
> the number of ports, because some ports have multiple flavours of interrupt
> dispatch code.

What exactly does the predicate say? "This cpu is running on it's local
interrupt stack"? "We are in the bottom half of the kernel"? "We took
some device interrupt and have not yet returned"?

Martin

Andrew Doran

unread,
Dec 6, 2007, 10:12:49 AM12/6/07
to
On Thu, Dec 06, 2007 at 03:31:04PM +0100, Martin Husemann wrote:

> On Thu, Dec 06, 2007 at 02:24:53PM +0000, Andrew Doran wrote:

> > It would break again the next time it is enabled. It also goes a bit beyond
> > the number of ports, because some ports have multiple flavours of interrupt
> > dispatch code.
>

> What exactly does the predicate say? "This cpu is running on it's local
> interrupt stack"? "We are in the bottom half of the kernel"? "We took
> some device interrupt and have not yet returned"?

The interrupt handling changes are described here:

http://www.netbsd.org/~ad/smp/vmlocking.txt:

Note that only the interrupt changes have gone into -HEAD so far. I filed
this PR a few days ago after looking at the interrupt path (although obviously
I missed adding cpu_intr_p() while there):

http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=37468

I have changed sparc to do the correct thing but sparc64 is missing the
calls. The simplest solution may be to add wrapper function if the interrupt
is registered at IPL_VM. x86 works like that:

http://nxr.netbsd.org/source/xref/sys/arch/x86/x86/intr.c#572
http://nxr.netbsd.org/source/xref/sys/arch/x86/x86/intr.c#611

Thanks,
Andrew

0 new messages