From owner-cvs-src@FreeBSD.ORG Tue Mar 28 19:51:49 2006 Return-Path: X-Original-To: cvs-src@freebsd.org Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C765216A72F; Tue, 28 Mar 2006 19:51:49 +0000 (UTC) (envelope-from des@des.no) Received: from tim.des.no (tim.des.no [194.63.250.121]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3578544416; Tue, 28 Mar 2006 19:28:10 +0000 (GMT) (envelope-from des@des.no) Received: from tim.des.no (localhost [127.0.0.1]) by spam.des.no (Postfix) with ESMTP id F0E4D2096; Tue, 28 Mar 2006 21:28:03 +0200 (CEST) X-Spam-Tests: AWL,BAYES_00,FORGED_RCVD_HELO X-Spam-Learn: ham X-Spam-Score: -2.4/3.0 X-Spam-Checker-Version: SpamAssassin 3.1.1 (2006-03-10) on tim.des.no Received: from xps.des.no (des.no [80.203.243.180]) by tim.des.no (Postfix) with ESMTP id 5D9342081; Tue, 28 Mar 2006 21:28:03 +0200 (CEST) Received: by xps.des.no (Postfix, from userid 1001) id 1F73B33C8D; Tue, 28 Mar 2006 21:28:03 +0200 (CEST) From: des@des.no (Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?=) To: John Baldwin References: <200603281434.k2SEYmaR031447@repoman.freebsd.org> <200603281300.09419.jhb@freebsd.org> <86psk6wilj.fsf@xps.des.no> <200603281358.26703.jhb@freebsd.org> Date: Tue, 28 Mar 2006 21:28:02 +0200 In-Reply-To: <200603281358.26703.jhb@freebsd.org> (John Baldwin's message of "Tue, 28 Mar 2006 13:58:24 -0500") Message-ID: <86acbawerh.fsf@xps.des.no> User-Agent: Gnus/5.110003 (No Gnus v0.3) Emacs/21.3 (berkeley-unix) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/i386/include atomic.h X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Mar 2006 19:51:50 -0000 --=-=-= Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable John Baldwin writes: > On Tuesday 28 March 2006 13:05, Dag-Erling Sm=F8rgrav wrote: > > John Baldwin writes: > > > One reason for not having the casts, btw, is that you lose type > > > checking. > > Huh? Before my patch, any use of atomic_*_ptr with warnings turned > > off would result in a slew of warnings because you'd be passing > > pointers to a function which is declared to take u_int. The only way > > to make this type safe is to use inline functions instead of the > > macros I wrote. > > s/off/on/ I trust > > Not true. The tinderbox would attest to that. Please see code such as > this: [...] which uses uintptr_t, not actual pointers, to avoid warnings. In effect, that code is broken. Apply the attached patch, see how far a buildkernel gets... I think the proper thing to do, to cover all your bases, would be to define a MD atomic_*_intptr family which operated on uintptr_t, and define an MI atomic_*_ptr family which operates on void * based on that. > Even userland uses casts when it uses void * rather than uintptr_t for > the underlying type. See src/lib/libpthread/sys/lock.c or > src/lib/libthr/thr_umtx.h. The latter only works because libthr is built with warnings disabled. I just finished working on making it build at WARNS level 2; higher levels will require a major overhaul, because the kernel interface it uses is fundamentally broken. DES --=20 Dag-Erling Sm=F8rgrav - des@des.no --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=i386-atomic.diff Index: sys/i386/include/atomic.h =================================================================== RCS file: /home/ncvs/src/sys/i386/include/atomic.h,v retrieving revision 1.42 diff -u -r1.42 atomic.h --- sys/i386/include/atomic.h 28 Mar 2006 14:34:48 -0000 1.42 +++ sys/i386/include/atomic.h 28 Mar 2006 19:24:17 -0000 @@ -415,42 +415,116 @@ #define atomic_fetchadd_32 atomic_fetchadd_int /* Operations on pointers. */ -#define atomic_set_ptr(p, v) \ - atomic_set_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_set_acq_ptr(p, v) \ - atomic_set_acq_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_set_rel_ptr(p, v) \ - atomic_set_rel_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_clear_ptr(p, v) \ - atomic_clear_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_clear_acq_ptr(p, v) \ - atomic_clear_acq_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_clear_rel_ptr(p, v) \ - atomic_clear_rel_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_add_ptr(p, v) \ - atomic_add_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_add_acq_ptr(p, v) \ - atomic_add_acq_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_add_rel_ptr(p, v) \ - atomic_add_rel_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_subtract_ptr(p, v) \ - atomic_subtract_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_subtract_acq_ptr(p, v) \ - atomic_subtract_acq_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_subtract_rel_ptr(p, v) \ - atomic_subtract_rel_int((volatile u_int *)(p), (u_int)(v)) -#define atomic_load_acq_ptr(p) \ - atomic_load_acq_int((volatile u_int *)(p)) -#define atomic_store_rel_ptr(p, v) \ - atomic_store_rel_int((volatile u_int *)(p), (v)) -#define atomic_cmpset_ptr(dst, old, new) \ - atomic_cmpset_int((volatile u_int *)(dst), (u_int)(old), (u_int)(new)) -#define atomic_cmpset_acq_ptr(dst, old, new) \ - atomic_cmpset_acq_int((volatile u_int *)(dst), (u_int)(old), (u_int)(new)) -#define atomic_cmpset_rel_ptr(dst, old, new) \ - atomic_cmpset_rel_int((volatile u_int *)(dst), (u_int)(old), (u_int)(new)) -#define atomic_readandclear_ptr(p) \ - atomic_readandclear_int((volatile u_int *)(p)) +/* Implemented as inline functions for the sake of type safety */ +static __inline void +atomic_set_ptr(volatile void **p, void *v) +{ + atomic_set_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_set_acq_ptr(volatile void **p, void *v) +{ + atomic_set_acq_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_set_rel_ptr(volatile void **p, void *v) +{ + atomic_set_rel_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_clear_ptr(volatile void **p, void *v) +{ + atomic_clear_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_clear_acq_ptr(volatile void **p, void *v) +{ + atomic_clear_acq_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_clear_rel_ptr(volatile void **p, void *v) +{ + atomic_clear_rel_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_add_ptr(volatile void **p, void *v) +{ + atomic_add_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_add_acq_ptr(volatile void **p, void *v) +{ + atomic_add_acq_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_add_rel_ptr(volatile void **p, void *v) +{ + atomic_add_rel_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_subtract_ptr(volatile void **p, void *v) +{ + atomic_subtract_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_subtract_acq_ptr(volatile void **p, void *v) +{ + atomic_subtract_acq_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void +atomic_subtract_rel_ptr(volatile void **p, void *v) +{ + atomic_subtract_rel_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline void * +atomic_load_acq_ptr(volatile void **p) +{ + return ((void *)(uintptr_t)atomic_load_acq_int((volatile uintptr_t *)p)); +} + +static __inline void +atomic_store_rel_ptr(volatile void **p, void *v) +{ + atomic_store_rel_int((volatile uintptr_t *)p, (uintptr_t)v); +} + +static __inline int +atomic_cmpset_ptr(volatile void **dst, void *old, void *new) +{ + return (atomic_cmpset_int((volatile uintptr_t *)dst, (uintptr_t)old, (uintptr_t)new)); +} + +static __inline int +atomic_cmpset_acq_ptr(volatile void **dst, void *old, void *new) +{ + return (atomic_cmpset_acq_int((volatile uintptr_t *)dst, + (uintptr_t)old, (uintptr_t)new)); +} + +static __inline int +atomic_cmpset_rel_ptr(volatile void **dst, void *old, void *new) +{ + return (atomic_cmpset_rel_int((volatile uintptr_t *)dst, + (uintptr_t)old, (uintptr_t)new)); +} + +static __inline void * +atomic_readandclear_ptr(volatile void **p) +{ + return ((void *)(uintptr_t)atomic_readandclear_int((volatile uintptr_t *)p)); +} #endif /* !defined(WANT_FUNCTIONS) */ #endif /* ! _MACHINE_ATOMIC_H_ */ --=-=-=--