Re: [head tinderbox] failure on sparc64/sun4v

On Wed, Jun 03, 2009 at 01:54:30PM +0400, Eygene Ryabinkin wrote:
> Wed, Jun 03, 2009 at 09:25:38AM +0400, Eygene Ryabinkin wrote:
> > Tue, Jun 02, 2009 at 06:24:45PM -0400, FreeBSD Tinderbox wrote:
> > > cc -c -O2 -pipe -fno-strict-aliasing  -std=c99  -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes  -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  -Wundef -Wno-pointer-sign -fformat-extensions -nostdinc  -I. -I/src/sys -I/src/sys/contrib/altq -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h -fno-common -finline-limit=15000 --param inline-unit-growth=100 --param large-function-growth=1000 -fno-builtin -mcmodel=medany -msoft-float -ffreestanding -fstack-protector -Werror  /src/sys/sun4v/sun4v/machdep.c
> > > /src/sys/sun4v/sun4v/machdep.c:192: error: size of array '__assert192' is negative
> > > *** Error code 1
> > > 
> > > Stop in /obj/sun4v/src/sys/LINT.
> > 
> > This seems to be related to the recent NETISR changes, namely, the
> > addition of the pc_netisr member to the struct pcpu:
> >   http://svn.freebsd.org/viewvc/base/head/sys/sys/pcpu.h?r1=187679&r2=193219&diff_format=u
> > 
> > I am not sure how large (void *) is on sun4v, but it seems to me
> > that it is 4 bytes long, so PCPU_MD_FIELDS_PAD inside sun4v/include/pcpu.h
>              ^^^^^^^^^^^^
> Sorry, eight bytes long: wrote 4, but really meant 8 ;))
> 
> > should be compensated for this change.  Something like
> > -----
> > #ifdef KTR
> > #define	PCPU_MD_FIELDS_PAD	(3 - (PCPU_NAME_LEN + 7) / 8)
> > #else
> > #define	PCPU_MD_FIELDS_PAD	3
> > #endif
> > -----
> > though I am not very sure about KTR's case.
> 
> KTR's case seems to be wrong for PCPU_NAME_LEN larger than 24 bytes.
> Just now we won't be able to reach this with the current definition
> for PCPU_NAME_LEN, but some day (N - (PCPU_NAME_LEN + 7)/8) can
> become negative and that's bad.

If it actually becomes negative the build should break again,
which IMO is sufficient protection.

> 
> The attached patch should fix this (although I have no sun4v to test
> on, so take it with a grain of salt).

I think this is overengineered, especially if not also
adjusting the padding for other macros which may change the
size of both MD and MI parts of struct pcpu.

> 
> By the way, having looked at sys/sys/pcpu.h, I see that there are parts
> of 'struct pcpu' that depend on the KTR_PERCPU being defined and they
> are never compensated with padding in PCPU_MD_FIELDS for sun4v.  Is
> KTR_PERCPU constant for sun4v (inexisting or defined everytime) or I am
> missing something?
> 

It's just not taken into account but AFAICT also dead code.

Marius