* [PATCH 0/2] makedumpfile: improve write_kdump_pages_cyclic @ 2022-04-04 15:50 Philipp Rudo 2022-04-04 15:50 ` [PATCH 1/2] makedumpfile: omit unnecessary calls to print_progress Philipp Rudo 2022-04-04 15:50 ` [PATCH 2/2] makedumpfile: break loop after last dumpable page Philipp Rudo 0 siblings, 2 replies; 8+ messages in thread From: Philipp Rudo @ 2022-04-04 15:50 UTC (permalink / raw) To: kexec Hi, I noticed that on s390 makedumpfile sometimes floods the console with Copying data : [100.0 %] - eta: 0s causing the dump process to slow down dramatically. The patches attached are two somewhat separate approaches to fix this issue. In my opinion both patches make sense to be included even when one of them would be enough to fix the problem described above. Thanks Philipp Philipp Rudo (2): makedumpfile: omit unnecessary calls to print_progress makedumpfile: break loop after last dumpable page makedumpfile.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] makedumpfile: omit unnecessary calls to print_progress 2022-04-04 15:50 [PATCH 0/2] makedumpfile: improve write_kdump_pages_cyclic Philipp Rudo @ 2022-04-04 15:50 ` Philipp Rudo 2022-04-07 6:42 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 2022-04-04 15:50 ` [PATCH 2/2] makedumpfile: break loop after last dumpable page Philipp Rudo 1 sibling, 1 reply; 8+ messages in thread From: Philipp Rudo @ 2022-04-04 15:50 UTC (permalink / raw) To: kexec Check first if a page is dumpable before printing the process. Otherwise there is the chance that num_dumped % per == 0 at the beginning of the block of undampable pages. In that case num_dumped isn't updated before the next dumpable page and thus print_process is called for every page in that block. This is especially annoying when the block is after the last dumpable page and thus num_dumped == info->num_dumpable. In that case print_process will bypass its check to only print the process once every second and thus flood the console with unnecessary prints. This can lead to a severe decrease in performance especially when the console is in line mode. Signed-off-by: Philipp Rudo <prudo@redhat.com> --- makedumpfile.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 14556db..2ef3458 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -8884,16 +8884,16 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag for (pfn = start_pfn; pfn < end_pfn; pfn++) { - if ((num_dumped % per) == 0) - print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable, &ts_start); - /* * Check the excluded page. */ if (!is_dumpable(info->bitmap2, pfn, cycle)) continue; + if ((num_dumped % per) == 0) + print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable, &ts_start); num_dumped++; + if (!read_pfn(pfn, buf)) goto out; -- 2.35.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/2] makedumpfile: omit unnecessary calls to print_progress 2022-04-04 15:50 ` [PATCH 1/2] makedumpfile: omit unnecessary calls to print_progress Philipp Rudo @ 2022-04-07 6:42 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 0 siblings, 0 replies; 8+ messages in thread From: HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= @ 2022-04-07 6:42 UTC (permalink / raw) To: kexec Hi Philipp, -----Original Message----- > Check first if a page is dumpable before printing the process. Otherwise > there is the chance that num_dumped % per == 0 at the beginning of the > block of undampable pages. In that case num_dumped isn't updated before > the next dumpable page and thus print_process is called for every page > in that block. > > This is especially annoying when the block is after the last dumpable > page and thus num_dumped == info->num_dumpable. In that case > print_process will bypass its check to only print the process once every > second and thus flood the console with unnecessary prints. This can lead > to a severe decrease in performance especially when the console is in > line mode. Good catch and improvement, will merge this. Thanks, Kazu > > Signed-off-by: Philipp Rudo <prudo@redhat.com> > --- > makedumpfile.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/makedumpfile.c b/makedumpfile.c > index 14556db..2ef3458 100644 > --- a/makedumpfile.c > +++ b/makedumpfile.c > @@ -8884,16 +8884,16 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag > > for (pfn = start_pfn; pfn < end_pfn; pfn++) { > > - if ((num_dumped % per) == 0) > - print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable, &ts_start); > - > /* > * Check the excluded page. > */ > if (!is_dumpable(info->bitmap2, pfn, cycle)) > continue; > > + if ((num_dumped % per) == 0) > + print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable, &ts_start); > num_dumped++; > + > if (!read_pfn(pfn, buf)) > goto out; > > -- > 2.35.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] makedumpfile: break loop after last dumpable page 2022-04-04 15:50 [PATCH 0/2] makedumpfile: improve write_kdump_pages_cyclic Philipp Rudo 2022-04-04 15:50 ` [PATCH 1/2] makedumpfile: omit unnecessary calls to print_progress Philipp Rudo @ 2022-04-04 15:50 ` Philipp Rudo 2022-04-07 6:43 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 1 sibling, 1 reply; 8+ messages in thread From: Philipp Rudo @ 2022-04-04 15:50 UTC (permalink / raw) To: kexec Once the last dumpable page was processed there is no need to finish the loop to the last page. Thus exit early to improve performance. Signed-off-by: Philipp Rudo <prudo@redhat.com> --- makedumpfile.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/makedumpfile.c b/makedumpfile.c index 2ef3458..c944d0e 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -8884,6 +8884,12 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag for (pfn = start_pfn; pfn < end_pfn; pfn++) { + /* + * All dumpable pages have been processed. No need to continue. + */ + if (num_dumped == info->num_dumpable) + break; + /* * Check the excluded page. */ -- 2.35.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] makedumpfile: break loop after last dumpable page 2022-04-04 15:50 ` [PATCH 2/2] makedumpfile: break loop after last dumpable page Philipp Rudo @ 2022-04-07 6:43 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 2022-04-07 6:51 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 2022-04-07 12:50 ` Philipp Rudo 0 siblings, 2 replies; 8+ messages in thread From: HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= @ 2022-04-07 6:43 UTC (permalink / raw) To: kexec -----Original Message----- > Once the last dumpable page was processed there is no need to finish the > loop to the last page. Thus exit early to improve performance. > > Signed-off-by: Philipp Rudo <prudo@redhat.com> > --- > makedumpfile.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/makedumpfile.c b/makedumpfile.c > index 2ef3458..c944d0e 100644 > --- a/makedumpfile.c > +++ b/makedumpfile.c > @@ -8884,6 +8884,12 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag > > for (pfn = start_pfn; pfn < end_pfn; pfn++) { > > + /* > + * All dumpable pages have been processed. No need to continue. > + */ > + if (num_dumped == info->num_dumpable) > + break; This patch is likely to increase the possibility of failure to capture /proc/kcore, although this is an unofficial functionality... # makedumpfile -ld31 /proc/kcore kcore.snap # crash vmlinux kcore.snap ... crash: page incomplete: kernel virtual address: ffff916fbeffed00 type: "pglist node_id" In cyclic mode, makedumpfile first calculates only info->num_dumpable [1] and frees the used bitmap, and later creates 2nd bitmap again [2] at this time. create_dumpfile create_dump_bitmap info->num_dumpable = get_num_dumpable_cyclic() <<-- [1] writeout_dumpfile write_kdump_pages_and_bitmap_cyclic foreach cycle create_2nd_bitmap <<-- [2] write_kdump_pages_cyclic So with live system, num_dumped can exceed info->num_dumpable. If it stops at info->num_dumpable, some necessary data can be missed. Capturing /proc/kcore is still fragile and not considered enough, but sometimes useful... when I want to capture a snapshot of memory. (the bitmap is allocated as block, so probably it's working as some buffer?) So I will merge the 1/2 patch, but personally would not like to merge this patch. How necessary is this? Thanks, Kazu ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] makedumpfile: break loop after last dumpable page 2022-04-07 6:43 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= @ 2022-04-07 6:51 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 2022-04-07 12:50 ` Philipp Rudo 1 sibling, 0 replies; 8+ messages in thread From: HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= @ 2022-04-07 6:51 UTC (permalink / raw) To: kexec -----Original Message----- > -----Original Message----- > > Once the last dumpable page was processed there is no need to finish the > > loop to the last page. Thus exit early to improve performance. > > > > Signed-off-by: Philipp Rudo <prudo@redhat.com> > > --- > > makedumpfile.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/makedumpfile.c b/makedumpfile.c > > index 2ef3458..c944d0e 100644 > > --- a/makedumpfile.c > > +++ b/makedumpfile.c > > @@ -8884,6 +8884,12 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag > > > > for (pfn = start_pfn; pfn < end_pfn; pfn++) { > > > > + /* > > + * All dumpable pages have been processed. No need to continue. > > + */ > > + if (num_dumped == info->num_dumpable) > > + break; > > This patch is likely to increase the possibility of failure to capture > /proc/kcore, although this is an unofficial functionality... > > # makedumpfile -ld31 /proc/kcore kcore.snap > # crash vmlinux kcore.snap > ... > crash: page incomplete: kernel virtual address: ffff916fbeffed00 type: "pglist node_id" > > In cyclic mode, makedumpfile first calculates only info->num_dumpable [1] and > frees the used bitmap, and later creates 2nd bitmap again [2] at this time. > > create_dumpfile > create_dump_bitmap > info->num_dumpable = get_num_dumpable_cyclic() <<-- [1] > writeout_dumpfile > write_kdump_pages_and_bitmap_cyclic > foreach cycle > create_2nd_bitmap <<-- [2] > write_kdump_pages_cyclic > > So with live system, num_dumped can exceed info->num_dumpable. > If it stops at info->num_dumpable, some necessary data can be missed. > > Capturing /proc/kcore is still fragile and not considered enough, but > sometimes useful... when I want to capture a snapshot of memory. > > (the bitmap is allocated as block, so probably it's working as some buffer?) sorry, mistake. (page descriptors are aligned by block, ...) > > So I will merge the 1/2 patch, but personally would not like to merge > this patch. How necessary is this? > > Thanks, > Kazu > > > _______________________________________________ > kexec mailing list > kexec at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] makedumpfile: break loop after last dumpable page 2022-04-07 6:43 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 2022-04-07 6:51 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= @ 2022-04-07 12:50 ` Philipp Rudo 2022-04-08 3:00 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 1 sibling, 1 reply; 8+ messages in thread From: Philipp Rudo @ 2022-04-07 12:50 UTC (permalink / raw) To: kexec Hi Kazu, On Thu, 7 Apr 2022 06:43:00 +0000 HAGIO KAZUHITO(?????) <k-hagio-ab@nec.com> wrote: > -----Original Message----- > > Once the last dumpable page was processed there is no need to finish the > > loop to the last page. Thus exit early to improve performance. > > > > Signed-off-by: Philipp Rudo <prudo@redhat.com> > > --- > > makedumpfile.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/makedumpfile.c b/makedumpfile.c > > index 2ef3458..c944d0e 100644 > > --- a/makedumpfile.c > > +++ b/makedumpfile.c > > @@ -8884,6 +8884,12 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag > > > > for (pfn = start_pfn; pfn < end_pfn; pfn++) { > > > > + /* > > + * All dumpable pages have been processed. No need to continue. > > + */ > > + if (num_dumped == info->num_dumpable) > > + break; > > This patch is likely to increase the possibility of failure to capture > /proc/kcore, although this is an unofficial functionality... > > # makedumpfile -ld31 /proc/kcore kcore.snap > # crash vmlinux kcore.snap > ... > crash: page incomplete: kernel virtual address: ffff916fbeffed00 type: "pglist node_id" > > In cyclic mode, makedumpfile first calculates only info->num_dumpable [1] and > frees the used bitmap, and later creates 2nd bitmap again [2] at this time. > > create_dumpfile > create_dump_bitmap > info->num_dumpable = get_num_dumpable_cyclic() <<-- [1] > writeout_dumpfile > write_kdump_pages_and_bitmap_cyclic > foreach cycle > create_2nd_bitmap <<-- [2] > write_kdump_pages_cyclic > > So with live system, num_dumped can exceed info->num_dumpable. > If it stops at info->num_dumpable, some necessary data can be missed. thanks for the explanation! I haven't considered that case and assumed info->num_dumpable is constant. > Capturing /proc/kcore is still fragile and not considered enough, but > sometimes useful... when I want to capture a snapshot of memory. > > (the bitmap is allocated as block, so probably it's working as some buffer?) > > So I will merge the 1/2 patch, but personally would not like to merge > this patch. How necessary is this? I don't think this patch is absolutely necessary. It's only a small performance improvement but shouldn't have a huge impact. If you like you can drop the patch. Thanks Philipp ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] makedumpfile: break loop after last dumpable page 2022-04-07 12:50 ` Philipp Rudo @ 2022-04-08 3:00 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 0 siblings, 0 replies; 8+ messages in thread From: HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= @ 2022-04-08 3:00 UTC (permalink / raw) To: kexec -----Original Message----- > Hi Kazu, > > On Thu, 7 Apr 2022 06:43:00 +0000 > HAGIO KAZUHITO(?????) <k-hagio-ab@nec.com> wrote: > > > -----Original Message----- > > > Once the last dumpable page was processed there is no need to finish the > > > loop to the last page. Thus exit early to improve performance. > > > > > > Signed-off-by: Philipp Rudo <prudo@redhat.com> > > > --- > > > makedumpfile.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/makedumpfile.c b/makedumpfile.c > > > index 2ef3458..c944d0e 100644 > > > --- a/makedumpfile.c > > > +++ b/makedumpfile.c > > > @@ -8884,6 +8884,12 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag > > > > > > for (pfn = start_pfn; pfn < end_pfn; pfn++) { > > > > > > + /* > > > + * All dumpable pages have been processed. No need to continue. > > > + */ > > > + if (num_dumped == info->num_dumpable) > > > + break; > > > > This patch is likely to increase the possibility of failure to capture > > /proc/kcore, although this is an unofficial functionality... > > > > # makedumpfile -ld31 /proc/kcore kcore.snap > > # crash vmlinux kcore.snap > > ... > > crash: page incomplete: kernel virtual address: ffff916fbeffed00 type: "pglist node_id" > > > > In cyclic mode, makedumpfile first calculates only info->num_dumpable [1] and > > frees the used bitmap, and later creates 2nd bitmap again [2] at this time. > > > > create_dumpfile > > create_dump_bitmap > > info->num_dumpable = get_num_dumpable_cyclic() <<-- [1] > > writeout_dumpfile > > write_kdump_pages_and_bitmap_cyclic > > foreach cycle > > create_2nd_bitmap <<-- [2] > > write_kdump_pages_cyclic > > > > So with live system, num_dumped can exceed info->num_dumpable. > > If it stops at info->num_dumpable, some necessary data can be missed. > > thanks for the explanation! I haven't considered that case and assumed > info->num_dumpable is constant. > > > Capturing /proc/kcore is still fragile and not considered enough, but > > sometimes useful... when I want to capture a snapshot of memory. > > > > (the bitmap is allocated as block, so probably it's working as some buffer?) > > > > So I will merge the 1/2 patch, but personally would not like to merge > > this patch. How necessary is this? > > I don't think this patch is absolutely necessary. It's only a small > performance improvement but shouldn't have a huge impact. If you like > you can drop the patch. ok, drop it at present. Applied the 1/2 patch. https://github.com/makedumpfile/makedumpfile/commit/3318c51c3c60ed192519cef6f6e72178151f88a8 Thanks! Kazu ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-04-08 3:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-04 15:50 [PATCH 0/2] makedumpfile: improve write_kdump_pages_cyclic Philipp Rudo 2022-04-04 15:50 ` [PATCH 1/2] makedumpfile: omit unnecessary calls to print_progress Philipp Rudo 2022-04-07 6:42 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 2022-04-04 15:50 ` [PATCH 2/2] makedumpfile: break loop after last dumpable page Philipp Rudo 2022-04-07 6:43 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 2022-04-07 6:51 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?= 2022-04-07 12:50 ` Philipp Rudo 2022-04-08 3:00 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?=
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.