From owner-freebsd-bugs@FreeBSD.ORG Fri Oct 8 20:16:52 2010 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 646EF106564A; Fri, 8 Oct 2010 20:16:52 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail08.syd.optusnet.com.au (mail08.syd.optusnet.com.au [211.29.132.189]) by mx1.freebsd.org (Postfix) with ESMTP id 002078FC19; Fri, 8 Oct 2010 20:16:51 +0000 (UTC) Received: from c122-106-146-165.carlnfd1.nsw.optusnet.com.au (c122-106-146-165.carlnfd1.nsw.optusnet.com.au [122.106.146.165]) by mail08.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o98KGhrJ031998 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 9 Oct 2010 07:16:49 +1100 Date: Sat, 9 Oct 2010 07:16:43 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Svatopluk Kraus In-Reply-To: <201010081226.o98CQbBI024222@www.freebsd.org> Message-ID: <20101009064440.Q1138@besplex.bde.org> References: <201010081226.o98CQbBI024222@www.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-bugs@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: kern/151304: patch - definitions of variables tested by ASSERT_ATOMIC_LOAD_PTR X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Oct 2010 20:16:52 -0000 On Fri, 8 Oct 2010, Svatopluk Kraus wrote: >> Description: > A macro ASSERT_ATOMIC_LOAD_PTR() hits in colfire port I work on. It is possibly due to use of GNU GCC (4.5.1) compiler -Os option (size optimalization). The macro is applied on four places: Perhaps gcc-4.5.1 -Os is doing invalid packing of structs, or FreeBSD has broken packing of structs using the __packed mistake and gcc-4.5.1 -Os exposes the brokenness by exploiting __packed more. Probably the latter. BTW, gcc-4.2.1 -Os is still completely broken on kernels. It fails to compile some files due to problems with -Wuninitialized, and when this is worked around it produces a kernel that is about 50% larger than one produced by gcc-4.2.1 -O. A few years ago I thought the problem was excessive inlining, but when I tried to fix it a few weeks ago, reducing the inlining caused larger problems and still gave large negative optimizations. >> Fix: > I patch the entries definitions in structures by align attribute, I believe it is better than to do nothing. Moreover, it solved my problem. > > Patch attached with submission follows: > > Index: sys/sys/_rwlock.h > =================================================================== > --- sys/sys/_rwlock.h (revision 213567) > +++ sys/sys/_rwlock.h (working copy) > @@ -37,7 +37,7 @@ > */ > struct rwlock { > struct lock_object lock_object; > - volatile uintptr_t rw_lock; > + volatile uintptr_t rw_lock __aligned(4); > }; > > #endif /* !_SYS__RWLOCK_H_ */ > ... This does nothing good on arches with 64-bit pointers. With gcc-4.2.1 on amd64, it seems to do nothing -- the natural alignment of a uintptr_t on amd64 is 8, and asking for a smaller alignment using __aligned(4) apparently makes no difference. It takes __aligned(more_than_8) or __packed to make a difference. Here is an example of creating an alignment bug due using __packed and just gcc-4.2.1 on amd64: % #include % #include % % struct foo { % int x; % long y __aligned(4); % }; % % struct bar { % char c; % struct foo d; % } __packed; % % int z = offsetof(struct foo, y); % int t = sizeof (struct bar); 'y' has the correct offset of 8, but `struct bar' has size 17, so the `y' nested in it cannot possibly be aligned properly, at least if there is an array of `struct bar' with at least 2 elements. I think this is a compiler bug -- the explicit __aligned(4) in the nested struct should force alignment of container structs (but only to 4 here, not the 8 that is required). Bruce