Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 4 Jun 2009 08:16:50 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        freebsd-current@freebsd.org
Cc:        kmacy@freebsd.org, marius@freebsd.org, Marcel Moolenaar <xcllnt@mac.com>, Robert Watson <rwatson@freebsd.org>, FreeBSD Tinderbox <tinderbox@freebsd.org>, sparc64@freebsd.org, Eygene Ryabinkin <rea-fbsd@codelabs.ru>, current@freebsd.org
Subject:   Re: [head tinderbox] failure on sparc64/sun4v
Message-ID:  <200906040816.51244.jhb@freebsd.org>
In-Reply-To: <15550775-6B8A-414E-A579-8B518D62E06E@mac.com>
References:  <20090602222445.2F6017302F@freebsd-current.sentex.ca> <alpine.BSF.2.00.0906032104100.74158@fledge.watson.org> <15550775-6B8A-414E-A579-8B518D62E06E@mac.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 03 June 2009 7:14:38 pm Marcel Moolenaar wrote:
> 
> On Jun 3, 2009, at 1:06 PM, Robert Watson wrote:
> >
> > Is there a reason not just to use __aligned(64) or the like on the  
> > first entry of the MD PCPU structure for sun4v to avoid future MI  
> > pcpu changes from causing similar discomfort for the MD pcpu parts?   
> > Also, do we know why these alignment/sizing requirements exist for  
> > struct pcpu on sun4v but not other platforms?  If this is about  
> > packing pcpu structures into properly aligned cache lines, again  
> > __aligned() might be the right approach to take...
> 
> Adding __aligned(xx) doesn't make it aligned. For example,
> malloc(3) only aligns at 16-byte boundaries, so any
> user-space structure that has __aligned(x>16) must manually
> make sure that this is actually the case by over-allocating
> and then adjusting the pointer to an x>16 aligned address.
> Likewise for the kernel, though it's easier in the kernel
> to get something that's page-aligned...
> FYI,

Yes, but struct pcpu is a bit special I think.  The MD code is responsible for 
allocating it (and in at least some cases it just allocates a complete page).  
As long as sun4v allocates struct pcpu on a 64 byte boundary, simply throwing 
__aligned() in will fix it.  Also, since the existing code is just computing 
explicit padding space, it is already assuming the alignment of struct pcpu.  
It is simply implementing __aligned() in a harder-to-maintain way.  It also 
doesn't make any sense the way it is doing it now since it is simply adding 
padding to the end of the structure.  Perhaps it is attempting to round up 
the size of the entire structure?  If so, that can easily be fixed in the MD 
code that allocates the structures by doing 'roundup(sizeof(struct pcpu), N)' 
when allocating the structure.

The comments in pcpu.h seem to imply that it wants a section of the fields in 
the middle to be aligned to a certain boundary in which case I think the 
proper solution is to stick an __aligned() on the first such member and then 
to allocate the structures on a suitable alignment when allocating PCPU 
structures in the MD code.  Presumably it would need the special work when 
allocating these structures even with the current hard-to-maintain padding 
hack.  Again, that hack is no better than __aligned(), just a much bigger 
pain to maintain AFAICT.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906040816.51244.jhb>