Re: [Crash-utility] Increase of NR_CPUS on IA64
Bernhard Walle wrote:
* Dave Anderson <anderson(a)redhat.com> [2007-10-19 18:23]:
>Bernhard Walle wrote:
>
>>* Bernhard Walle <bwalle(a)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