Re: amd64 atomic macro naming cleanup

Previous thread: cleanup directory sys/dev/pckbc by Alexandr Shadchin on Saturday, December 25, 2010 - 10:57 am. (2 messages)

Next thread: correct mxcsr+mxcsr_mask handling (revised) by Philip Guenther on Saturday, December 25, 2010 - 1:02 pm. (7 messages)
From: Philip Guenther
Date: Saturday, December 25, 2010 - 12:12 pm

Fix the naming of the atomic macros on amd64: right now, the 
x86_atomic_*_l() macros actually operate on unsigned 32bit integers 
instead of longs, so:
1) change the callers to use the _u32 versions
2) update the _ul definitions to bew the 64bit versions
3) remove the _l macros

Open question: perhaps the _ul macros should be removed too (they're 
unused after this diff) and future code should just explicitly use the 
_u32 or _u64 macros?


Also, complete remove the x86_multicast_ipi() function as unused (it was 
broken by the move to suport 64 cpus too).

oks?

Philip Guenther


Index: amd64/intr.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/intr.c,v
retrieving revision 1.25
diff -u -p -r1.25 intr.c
--- amd64/intr.c	20 Sep 2010 06:33:46 -0000	1.25
+++ amd64/intr.c	25 Dec 2010 17:46:01 -0000
@@ -498,7 +498,7 @@ intr_disestablish(struct intrhand *ih)
 
 	simple_lock(&ci->ci_slock);
 	pic->pic_hwmask(pic, ih->ih_pin);	
-	x86_atomic_clearbits_l(&ci->ci_ipending, (1 << ih->ih_slot));
+	x86_atomic_clearbits_u32(&ci->ci_ipending, (1 << ih->ih_slot));
 
 	/*
 	 * Remove the handler from the chain.
Index: amd64/ipi.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/ipi.c,v
retrieving revision 1.8
diff -u -p -r1.8 ipi.c
--- amd64/ipi.c	26 Jun 2008 05:42:09 -0000	1.8
+++ amd64/ipi.c	25 Dec 2010 17:46:01 -0000
@@ -50,7 +50,7 @@ x86_send_ipi(struct cpu_info *ci, int ip
 {
 	int ret;
 
-	x86_atomic_setbits_l(&ci->ci_ipis, ipimask);
+	x86_atomic_setbits_u32(&ci->ci_ipis, ipimask);
 
 	/* Don't send IPI to cpu which isn't (yet) running. */
 	if (!(ci->ci_flags & CPUF_RUNNING))
@@ -88,7 +88,7 @@ x86_broadcast_ipi(int ipimask)
 			continue;
 		if ((ci->ci_flags & CPUF_RUNNING) == 0)
 			continue;
-		x86_atomic_setbits_l(&ci->ci_ipis, ipimask);
+		x86_atomic_setbits_u32(&ci->ci_ipis, ipimask);
 		count++;
 	}
 	if (!count)
@@ -98,23 +98,6 @@ ...
From: Ted Unangst
Date: Saturday, December 25, 2010 - 12:35 pm

This looks good, but I think we should delete the ul macro too.  There
should be very few "long" types running around in the kernel.

From: Philip Guenther
Date: Sunday, December 26, 2010 - 9:26 pm

That's simple enough, it just means deleting three '+' lines from the
atomic.h part of the diff, deleting the _l and _ul macros and leaving
just the x86_atomic_testset_i() macro (which is only used in lock.h).
I'm fine with that.

oks?


Philip Guenther

From: Mark Kettenis
Date: Monday, December 27, 2010 - 9:07 am

Actually we have quite a bit of "long" usage in the kernel; addresses,
sizes, etc. etc.  Basically everything that needs to be 32 bits on a
32-bit system and 64 bits on a 64-bit system is defined as a typedef
of "long".

What are these x86_atomic_*() macros used for?  Well, we inherited
them from NetBSD, and there they are used to make sharing code between
i386 and amd64 easier.  Now in OpenBSD we don't really share code like
that, but I still think it should still be our goal to reduce the
diffs between our i386 and amd64 codebase as much as possible.  That
means "variable" "long" variants of the atomic functions are probably
needed in addition to the "fixed" 32-bit and 64-bit ones.

Ultimately we should probably define a constent MI set of atomic
interfaces.  But until then I think it makes sense to keep the _l and

So I think Philip's origional diff should go in.  The
x86_multicast_ipi() removal should probably be a seperate commit
though.

Previous thread: cleanup directory sys/dev/pckbc by Alexandr Shadchin on Saturday, December 25, 2010 - 10:57 am. (2 messages)

Next thread: correct mxcsr+mxcsr_mask handling (revised) by Philip Guenther on Saturday, December 25, 2010 - 1:02 pm. (7 messages)