On Fri, 4 Oct 2002, Randy.Dunlap wrote:
|>On Fri, 4 Oct 2002, Matt D. Robinson wrote:
|>| These are the patches (9 in all) for the Linux Kernel Crash Dump
|>| modifications for 2.5. This allows crash dumps to be built as a
|>| module in the kernel and also includes a breakdown of a few of the
|>| changes needed in the kernel. The current version will allow for
|>| block dumping, and has the ability to readily integrate network
|>| dumping (a la Red Hat).
|>
|>Hi Matt,
|>
|>I have a few comments.
|>
|>| Please accept these patches or provide feedback on how we can
|>| modify them for acceptance. Thanks,
|>
|>Who do you want to accept these patches?
|>If it's Linus, you should send them to him.
They're on the way (or will be later tonight after putting in
your requested changes).
|>Documentation/SubmittingPatches doesn't say so, but lately it's
|>become quite common and desirable to use diffstat output above
|>patches to summarize the files modified and how much they are
|>modified.
Okay, can do.
|>Instead of replying to other patches separately, I'll add a few
|>comments here.
|>
|>CONFIG_DUMP: I'd prefer to see something a little bit more descriptive,
|>like CONFIG_CRASH_DUMP or CONFIG_DUMP_KERNEL. Yes, this is minor.
|>(BTW, I don't like the shortness of CONFIG_TRACE for LTT either. :)
That's a fairly easy change. CONFIG_CRASH_DUMP is better over
the long run. I'll preface all the CONFIG_??* values. It really
doesn't matter one way or the other to me.
|>Why are all of the dump_init() and secondary init functions
|>_not_ marked as __init ?
|>
|>You shouldn't need to call dump_init() explicitly since you use
|>module_init(dump_init);
|>Oh, I see, you call dump_init() for built-into-kernel but use
|>#ifdef MODULE
|>to surround lots of MODULE_xyz() lines.
|>You shouldn't surround the MODULE_xyz() lines like that,
|>they should always be present for MODULE or not,
|>and you should just use the module_init(dump_init) always to
|>initialize.
Okay, simple enough to fix.
Thanks, Randy, I'll start incorporating those for Linus.
--Matt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon Oct 07 2002 - 22:00:49 EST