* [PATCH] makedumpfile: fix free partial_bitmap2 error
@ 2014-04-25 1:43 Arthur Zou
2014-04-28 6:21 ` Baoquan He
2014-04-28 7:28 ` Atsushi Kumagai
0 siblings, 2 replies; 4+ messages in thread
From: Arthur Zou @ 2014-04-25 1:43 UTC (permalink / raw)
To: kexec; +Cc: Arthur Zou, kumagai-atsushi
Description:
In create_dump_bitmap, after prepare_bitmap2_buffer_cyclic was invoked,
info->partial_bitmap2 will pointed to a block of contiguous memory. But
free it in a wrong way because what free_bitmap2_buffer() free is
info->bitmap2 not info->partial_bitmap2.
Signed-off-by: Arthur Zou <zzou@redhat.com>
---
makedumpfile.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index ce4a866..f0d2997 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -5143,7 +5143,8 @@ create_dump_bitmap(void)
info->num_dumpable = get_num_dumpable_cyclic();
- free_bitmap2_buffer();
+ if (info->partial_bitmap2 != NULL)
+ free(info->partial_bitmap2);
}
} else {
--
1.8.4.2
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] makedumpfile: fix free partial_bitmap2 error
2014-04-25 1:43 [PATCH] makedumpfile: fix free partial_bitmap2 error Arthur Zou
@ 2014-04-28 6:21 ` Baoquan He
2014-04-28 6:29 ` Baoquan He
2014-04-28 7:28 ` Atsushi Kumagai
1 sibling, 1 reply; 4+ messages in thread
From: Baoquan He @ 2014-04-28 6:21 UTC (permalink / raw)
To: Arthur Zou; +Cc: kumagai-atsushi, kexec
On 04/25/14 at 09:43am, Arthur Zou wrote:
> Description:
> In create_dump_bitmap, after prepare_bitmap2_buffer_cyclic was invoked,
> info->partial_bitmap2 will pointed to a block of contiguous memory. But
> free it in a wrong way because what free_bitmap2_buffer() free is
> info->bitmap2 not info->partial_bitmap2.
>
> Signed-off-by: Arthur Zou <zzou@redhat.com>
> ---
> makedumpfile.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/makedumpfile.c b/makedumpfile.c
> index ce4a866..f0d2997 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -5143,7 +5143,8 @@ create_dump_bitmap(void)
>
> info->num_dumpable = get_num_dumpable_cyclic();
>
> - free_bitmap2_buffer();
> + if (info->partial_bitmap2 != NULL)
> + free(info->partial_bitmap2);
ACK
Hi Atsushi,
Maybe in my case which the lzo dump random failure triggered by this
one. Because for elf dump, since the wrong calculation of
cyclic_bufsize is corrected, OOM never happened. For this bug, it didn't
happen either after the fix applied in this patch.
So I guess the 80% of free memory is a safe value, though it's very
close to the OOM threshold.
Thanks
Baoquan
> }
>
> } else {
> --
> 1.8.4.2
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] makedumpfile: fix free partial_bitmap2 error
2014-04-28 6:21 ` Baoquan He
@ 2014-04-28 6:29 ` Baoquan He
0 siblings, 0 replies; 4+ messages in thread
From: Baoquan He @ 2014-04-28 6:29 UTC (permalink / raw)
To: Arthur Zou; +Cc: kumagai-atsushi, kexec
On 04/28/14 at 02:21pm, Baoquan He wrote:
> On 04/25/14 at 09:43am, Arthur Zou wrote:
> > Description:
> > In create_dump_bitmap, after prepare_bitmap2_buffer_cyclic was invoked,
> > info->partial_bitmap2 will pointed to a block of contiguous memory. But
> > free it in a wrong way because what free_bitmap2_buffer() free is
> > info->bitmap2 not info->partial_bitmap2.
> >
> > Signed-off-by: Arthur Zou <zzou@redhat.com>
> > ---
> > makedumpfile.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/makedumpfile.c b/makedumpfile.c
> > index ce4a866..f0d2997 100644
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -5143,7 +5143,8 @@ create_dump_bitmap(void)
> >
> > info->num_dumpable = get_num_dumpable_cyclic();
> >
> > - free_bitmap2_buffer();
> > + if (info->partial_bitmap2 != NULL)
> > + free(info->partial_bitmap2);
>
> ACK
>
> Hi Atsushi,
>
> Maybe in my case which the lzo dump random failure triggered by this
> one. Because for elf dump, since the wrong calculation of
> cyclic_bufsize is corrected, OOM never happened. For this bug, it didn't
> happen either after the fix applied in this patch.
>
Well, I was wrong. It can't prove 80% of free memory is a safe value,
espeically for my case. Please ignore below paragraph, it's not clear in
my head.
> So I guess the 80% of free memory is a safe value, though it's very
> close to the OOM threshold.
>
> Thanks
> Baoquan
>
>
> > }
> >
> > } else {
> > --
> > 1.8.4.2
> >
> >
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] makedumpfile: fix free partial_bitmap2 error
2014-04-25 1:43 [PATCH] makedumpfile: fix free partial_bitmap2 error Arthur Zou
2014-04-28 6:21 ` Baoquan He
@ 2014-04-28 7:28 ` Atsushi Kumagai
1 sibling, 0 replies; 4+ messages in thread
From: Atsushi Kumagai @ 2014-04-28 7:28 UTC (permalink / raw)
To: zzou; +Cc: kexec
>Description:
>In create_dump_bitmap, after prepare_bitmap2_buffer_cyclic was invoked,
>info->partial_bitmap2 will pointed to a block of contiguous memory. But
>free it in a wrong way because what free_bitmap2_buffer() free is
>info->bitmap2 not info->partial_bitmap2.
>
>Signed-off-by: Arthur Zou <zzou@redhat.com>
Good catch, Thanks!
>---
> makedumpfile.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/makedumpfile.c b/makedumpfile.c
>index ce4a866..f0d2997 100644
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -5143,7 +5143,8 @@ create_dump_bitmap(void)
>
> info->num_dumpable = get_num_dumpable_cyclic();
>
>- free_bitmap2_buffer();
>+ if (info->partial_bitmap2 != NULL)
>+ free(info->partial_bitmap2);
I think it's better to create free_bitmap2_buffer_cyclic() for this.
BTW, write_kdump_pages_and_bitmap_cyclic() also have the same mistake:
struct cycle cycle = {0};
for_each_cycle(0, info->max_mapnr, &cycle)
{
if (!create_1st_bitmap_cyclic(&cycle))
return FALSE;
if (!write_kdump_bitmap1_cyclic(&cycle))
return FALSE;
}
free_bitmap1_buffer();
What we should do here is to free partial_bitmap1, so we should
create free_bitmap1_buffer_cyclic() and call it here instead.
Luckily, this mistake is harmless due to the free() at the end
of main():
if (info->partial_bitmap1 != NULL)
free(info->partial_bitmap1);
if (info->partial_bitmap2 != NULL)
free(info->partial_bitmap2);
Now, I think it's better to remove these free(), it's possible
if partial_bitmap1 and partial_bitmap2 are freed in
write_kdump_pages_and_bitmap_cyclic() or write_elf_pages_cyclic().
So could you fix your patch based on my comments?
Thanks
Atsushi Kumagai
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-04-28 7:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-25 1:43 [PATCH] makedumpfile: fix free partial_bitmap2 error Arthur Zou
2014-04-28 6:21 ` Baoquan He
2014-04-28 6:29 ` Baoquan He
2014-04-28 7:28 ` Atsushi Kumagai
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.