[Crash-utility] Increase of NR_CPUS on IA64

[Dave Anderson]

Bernhard Walle wrote:
> * Dave Anderson <anderson at redhat.com> [2007-10-19 18:23]:
> 
>>Bernhard Walle wrote:
>>
>>>* Bernhard Walle <bwalle at suse.de> [2007-10-19 17:24]:
>>>
>>>>The solution for that problem is to calculate the number of CPUs for
>>>>IA64 at runtime. The 2nd patch implements this, and also reads the
>>>>registers from the LKCD dump header instead of guessing on the stack.
>>>>This fixes a problem here -- unfortunately, I don't still have that
>>>>dump to provide further details.
>>>
>>>I forget to attach the 2nd patch ;)
>>
>>At this point I've lost most insights into the LKCD code.
>>The ia64 pt_regs hardwiring bothers me, mainly because there have
>>been changes to its definition over the years.  (There's a couple
>>different versions hardwired in unwind.h for example).
> 
> 
> In the last version of the patch (well, I know, it's 1/2 year now), I
> removed the hard wired pt_regs value. You or Troy (didn't remember)
> complained that this doesn't work *because* the definition changed
> over years. And here we rely on the pt_regs structure of the dumping
> kernel.

But as I recall, you #include'd ptrace.h, so the same
issue would arise, i.e., it would depend upon the build
environment, which may not match the host environment.

Screwed either way I guess...

> 
> 
>>So my biggest worry would be if this somehow breaks
>>backwards-compatibility, but I'm presuming that you took
>>that into account.  But anyway, I leave this all up
>>to Troy.
> 
> 
> Of course, that's possible. Always. But leaving existing bugs unfixed
> is also not really a good idea. However, as I said, I'm willing to
> split off the LKCD registers change (which is 90 %) and the dynamic
> CPU calculation (which is 10 %) so that only the last remaining 10 %
> can be included.
> 
> 
>>Please build with "make warn" before submitting any patch
>>and clean up the complaints.
> 
> 
> Thanks for the hint. I'll do this in future!
> 
> 
>>Also I'd prefer to not tinker with the netdump.c file.
>>There is no /usr/include/stddef.h in the RHEL and FC8
>>environments, and the /usr/include/linux/stddef.h has
>>removed offsetof() in FC8 for some reason?  In any case,
>>I'd prefer leaving it alone.
> 
> 
> I cannot imagine this. "man offsetof" gives me
> 
> SYNOPSIS
>        #include <stddef.h>
> 
>        size_t offsetof(type, member);
> 
> CONFORMING TO
>        C89, C99, POSIX.1-2001.
> 
> So either the manpage is completely wrong, you're wrong or FC8 and
> RHEL don't conform to the standards mentioned.

You're right, I'm wrong.  netdump.c compiles on FC8, although I'm
not sure where it gets offsetof() from.  Here is the FC8 version of
linux/stddef.h:

   #ifndef _LINUX_STDDEF_H
   #define _LINUX_STDDEF_H


   #undef NULL
   #if defined(__cplusplus)
   #define NULL 0
   #else
   #define NULL ((void *)0)
   #endif


   #endif

whereas it's there there in all earlier RHEL kernel versions,
and a compiler-sensitive version is there in upstream 2.6.22.6?

Whatever...

> 
> 
>>Because of the builtin array sizes that I long ago
>>painted crash into a corner with, I'd also prefer keeping
>>NR_CPUS to its minimum.  I have no problem updating it if
>>necessary later, so 4096 is preferable to overkilling
>>it with 16384 at this time. 
> 
> 
> Ok, then we update it to 4096. I have no objections. It wasn't even my
> idea to update it to 16384.
> 
> 
>>I don't see how that could be a problem?  If it is, you can always
>>release the SUSE version with the larger value -- I'm sure that
>>you're almost never in sync with the upstream version anyway, right?
>>(I can't even do that with RHEL errata...)
> 
> 
> No, I never sync to mainline for released SLES versions. However, I
> always try to keep the number of custom patches of Factory (latest
> openSUSE release) as small as possible because:
> 
>   o Less work on update. crash has frequent releases.
> 
>   o Easier to report bugs mainline. I even see that users/customers
>     ask on that mailing list. That's difficult if the SUSE crash
>     differs from the mainline crash too much.
> 

Yep...

> 
> 
> Thanks,
>    Bernhard