Jingbai Ma<jingbai.ma@xxxxxx> writes:
On 03/08/2013 11:52 PM, Vivek Goyal wrote:On Thu, Mar 07, 2013 at 01:54:45PM -0800, Eric W. Biederman wrote:Vivek Goyal<vgoyal@xxxxxxxxxx> writes:
On Thu, Mar 07, 2013 at 10:58:18PM +0800, Jingbai Ma wrote:This patch intend to speedup the memory pages scanning process in
selective dump mode.
Test result (On HP ProLiant DL980 G7 with 1TB RAM, makedumpfile
v1.5.3):
Total scan Time
Original kernel
+ makedumpfile v1.5.3 cyclic mode 1958.05 seconds
Original kernel
+ makedumpfile v1.5.3 non-cyclic mode 1151.50 seconds
Patched kernel
+ patched makedumpfile v1.5.3 17.50 seconds
Traditionally, to reduce the size of dump file, dumper scans all memory
pages to exclude the unnecessary memory pages after capture kernel
booted, and scan it in userspace code (makedumpfile).
I think this is not a good idea. It has several issues.
Actually it does not appear to be doing any work in the first kernel.
Looks like patch3 in series is doing that.
machine_crash_shutdown(&fixed_regs);
+ generate_crash_dump_bitmap();
machine_kexec(kexec_crash_image);
So this bitmap seems to be being set just before transitioning into
second kernel.
I am sure you would not like this extra code in this path. :-)
I was thought this function code is pretty simple, could be called
here safely.
If it's not proper for here, how about before the function
machine_crash_shutdown(&fixed_regs)?
Furthermore, could you explain the real risks to execute more codes here?
The kernel is known bad. What is bad is unclear.
Executing any extra code is a bad idea.
The history here is that before kexec-on-panic there were lots of dump
routines that did all of the crashdump logic in the kernel before they
shutdown. They all worked beautifully during development, and on
developers test machines and were absolutely worthless in real world
situations.
A piece of code that walks all of the page tables is most definitely
opening itself up to all kinds of failure situations I can't even
imagine.
The only way that it would be ok to do this would be to maintain the
bitmap in real time with the existing page table maintenance code,
and that would only be ok if it did not add a performance penalty.
Every once in a great while there is a new cpu architecture feature
we need to deal with, but otherwise the only thing that is ok to
do on that code path is to reduce it until it much more closely
resembles the glorified jump instruction that it really is.
Speaking of have you given this code any coverage testing with lkdtm?
Eric