From mboxrd@z Thu Jan 1 00:00:00 1970 From: Philipp Rudo Date: Thu, 7 Apr 2022 14:50:14 +0200 Subject: [PATCH 2/2] makedumpfile: break loop after last dumpable page In-Reply-To: References: <20220404155050.6898-1-prudo@redhat.com> <20220404155050.6898-3-prudo@redhat.com> Message-ID: <20220407145014.2785147a@rhtmp> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: kexec@lists.infradead.org Hi Kazu, On Thu, 7 Apr 2022 06:43:00 +0000 HAGIO KAZUHITO(?????) 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 > > --- > > 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