* [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated
@ 2020-01-20 2:33 Pingfan Liu
2020-01-20 7:29 ` Michal Hocko
0 siblings, 1 reply; 7+ messages in thread
From: Pingfan Liu @ 2020-01-20 2:33 UTC (permalink / raw)
To: linux-mm
Cc: Pingfan Liu, Andrew Morton, David Hildenbrand, Dan Williams,
Oscar Salvador, Michal Hocko, Baoquan He, Qian Cai, kexec,
Kazuhito Hagio
After commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug"),
when a mem section is fully deactivated, section_mem_map still records the
section's start pfn, which is not used any more and will be reassigned
during re-added.
In analogy with alloc/free pattern, it is better to clear all fields of
section_mem_map.
Beside this, it breaks the user space tool "makedumpfile" [1], which makes
assumption that a hot-removed section has mem_map as NULL, instead of
checking directly against SECTION_MARKED_PRESENT bit. (makedumpfile will be
better to change the assumption, and need a patch)
The bug can be reproduced on IBM POWERVM by "drmgr -c mem -r -q 5" ,
trigger a crash, and save vmcore by makedumpfile
[1]: makedumpfile, commit e73016540293 ("[v1.6.7] Update version")
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
To: linux-mm@kvack.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Qian Cai <cai@lca.pw>
Cc: kexec@lists.infradead.org
Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
---
v1 -> v2:
make an explicit convertion from NULL to ulong
improve commit log
mm/sparse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/sparse.c b/mm/sparse.c
index 3822ecb..3918fc3 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -789,7 +789,7 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
ms->usage = NULL;
}
memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
- ms->section_mem_map = sparse_encode_mem_map(NULL, section_nr);
+ ms->section_mem_map = (unsigned long)NULL;
}
if (section_is_early && memmap)
--
2.7.5
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated
2020-01-20 2:33 [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated Pingfan Liu
@ 2020-01-20 7:29 ` Michal Hocko
2020-01-20 9:03 ` David Hildenbrand
2020-01-24 3:10 ` Andrew Morton
0 siblings, 2 replies; 7+ messages in thread
From: Michal Hocko @ 2020-01-20 7:29 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-mm, Andrew Morton, David Hildenbrand, Dan Williams,
Oscar Salvador, Baoquan He, Qian Cai, kexec, Kazuhito Hagio
On Mon 20-01-20 10:33:14, Pingfan Liu wrote:
> After commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug"),
> when a mem section is fully deactivated, section_mem_map still records the
> section's start pfn, which is not used any more and will be reassigned
> during re-added.
>
> In analogy with alloc/free pattern, it is better to clear all fields of
> section_mem_map.
>
> Beside this, it breaks the user space tool "makedumpfile" [1], which makes
> assumption that a hot-removed section has mem_map as NULL, instead of
> checking directly against SECTION_MARKED_PRESENT bit. (makedumpfile will be
> better to change the assumption, and need a patch)
>
> The bug can be reproduced on IBM POWERVM by "drmgr -c mem -r -q 5" ,
> trigger a crash, and save vmcore by makedumpfile
While makedumpfile lives very closely to the kernel and occasional
breakage is to be expected I still believe that Fixes: ba72b4c8cf60
is due.
> [1]: makedumpfile, commit e73016540293 ("[v1.6.7] Update version")
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> To: linux-mm@kvack.org
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: kexec@lists.infradead.org
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> v1 -> v2:
> make an explicit convertion from NULL to ulong
> improve commit log
>
> mm/sparse.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 3822ecb..3918fc3 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -789,7 +789,7 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> ms->usage = NULL;
> }
> memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> - ms->section_mem_map = sparse_encode_mem_map(NULL, section_nr);
> + ms->section_mem_map = (unsigned long)NULL;
> }
>
> if (section_is_early && memmap)
> --
> 2.7.5
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated
2020-01-20 7:29 ` Michal Hocko
@ 2020-01-20 9:03 ` David Hildenbrand
2020-01-20 10:12 ` Pingfan Liu
2020-01-24 3:10 ` Andrew Morton
1 sibling, 1 reply; 7+ messages in thread
From: David Hildenbrand @ 2020-01-20 9:03 UTC (permalink / raw)
To: Michal Hocko, Pingfan Liu
Cc: linux-mm, Andrew Morton, Dan Williams, Oscar Salvador,
Baoquan He, Qian Cai, kexec, Kazuhito Hagio
On 20.01.20 08:29, Michal Hocko wrote:
> On Mon 20-01-20 10:33:14, Pingfan Liu wrote:
>> After commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug"),
>> when a mem section is fully deactivated, section_mem_map still records the
>> section's start pfn, which is not used any more and will be reassigned
>> during re-added.
>>
>> In analogy with alloc/free pattern, it is better to clear all fields of
>> section_mem_map.
>>
>> Beside this, it breaks the user space tool "makedumpfile" [1], which makes
>> assumption that a hot-removed section has mem_map as NULL, instead of
>> checking directly against SECTION_MARKED_PRESENT bit. (makedumpfile will be
>> better to change the assumption, and need a patch)
>>
>> The bug can be reproduced on IBM POWERVM by "drmgr -c mem -r -q 5" ,
>> trigger a crash, and save vmcore by makedumpfile
>
> While makedumpfile lives very closely to the kernel and occasional
> breakage is to be expected I still believe that Fixes: ba72b4c8cf60
> is due.
+1 as I expressed earlier.
>
>> [1]: makedumpfile, commit e73016540293 ("[v1.6.7] Update version")
>>
>> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
>> To: linux-mm@kvack.org
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Oscar Salvador <osalvador@suse.de>
>> Cc: Michal Hocko <mhocko@kernel.org>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Qian Cai <cai@lca.pw>
>> Cc: kexec@lists.infradead.org
>> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
>
I think you dropped my
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated
2020-01-20 9:03 ` David Hildenbrand
@ 2020-01-20 10:12 ` Pingfan Liu
0 siblings, 0 replies; 7+ messages in thread
From: Pingfan Liu @ 2020-01-20 10:12 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michal Hocko, Linux-MM, Andrew Morton, Dan Williams,
Oscar Salvador, Baoquan He, Qian Cai, Kexec Mailing List,
Kazuhito Hagio
On Mon, Jan 20, 2020 at 5:03 PM David Hildenbrand <david@redhat.com> wrote:
>
[...]
> I think you dropped my
>
> Acked-by: David Hildenbrand <david@redhat.com>
>
Sorry to forget it.
Thanks for your kind review.
Regards,
Pingfan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated
2020-01-20 7:29 ` Michal Hocko
2020-01-20 9:03 ` David Hildenbrand
@ 2020-01-24 3:10 ` Andrew Morton
2020-01-24 6:49 ` Michal Hocko
1 sibling, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2020-01-24 3:10 UTC (permalink / raw)
To: Michal Hocko
Cc: Pingfan Liu, linux-mm, David Hildenbrand, Dan Williams,
Oscar Salvador, Baoquan He, Qian Cai, kexec, Kazuhito Hagio
On Mon, 20 Jan 2020 08:29:39 +0100 Michal Hocko <mhocko@suse.com> wrote:
> On Mon 20-01-20 10:33:14, Pingfan Liu wrote:
> > After commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug"),
> > when a mem section is fully deactivated, section_mem_map still records the
> > section's start pfn, which is not used any more and will be reassigned
> > during re-added.
> >
> > In analogy with alloc/free pattern, it is better to clear all fields of
> > section_mem_map.
> >
> > Beside this, it breaks the user space tool "makedumpfile" [1], which makes
> > assumption that a hot-removed section has mem_map as NULL, instead of
> > checking directly against SECTION_MARKED_PRESENT bit. (makedumpfile will be
> > better to change the assumption, and need a patch)
> >
> > The bug can be reproduced on IBM POWERVM by "drmgr -c mem -r -q 5" ,
> > trigger a crash, and save vmcore by makedumpfile
>
> While makedumpfile lives very closely to the kernel and occasional
> breakage is to be expected I still believe that Fixes: ba72b4c8cf60
> is due.
But not a cc:stable?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated
2020-01-24 3:10 ` Andrew Morton
@ 2020-01-24 6:49 ` Michal Hocko
2020-01-25 13:26 ` Pingfan Liu
0 siblings, 1 reply; 7+ messages in thread
From: Michal Hocko @ 2020-01-24 6:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Pingfan Liu, linux-mm, David Hildenbrand, Dan Williams,
Oscar Salvador, Baoquan He, Qian Cai, kexec, Kazuhito Hagio
On Thu 23-01-20 19:10:47, Andrew Morton wrote:
> On Mon, 20 Jan 2020 08:29:39 +0100 Michal Hocko <mhocko@suse.com> wrote:
>
> > On Mon 20-01-20 10:33:14, Pingfan Liu wrote:
> > > After commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug"),
> > > when a mem section is fully deactivated, section_mem_map still records the
> > > section's start pfn, which is not used any more and will be reassigned
> > > during re-added.
> > >
> > > In analogy with alloc/free pattern, it is better to clear all fields of
> > > section_mem_map.
> > >
> > > Beside this, it breaks the user space tool "makedumpfile" [1], which makes
> > > assumption that a hot-removed section has mem_map as NULL, instead of
> > > checking directly against SECTION_MARKED_PRESENT bit. (makedumpfile will be
> > > better to change the assumption, and need a patch)
> > >
> > > The bug can be reproduced on IBM POWERVM by "drmgr -c mem -r -q 5" ,
> > > trigger a crash, and save vmcore by makedumpfile
> >
> > While makedumpfile lives very closely to the kernel and occasional
> > breakage is to be expected I still believe that Fixes: ba72b4c8cf60
> > is due.
>
> But not a cc:stable?
Well, I wouldn't say this is really critical. makedumpfile will get its
fix... But if people think it would be useful in stable I won't oppose.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated
2020-01-24 6:49 ` Michal Hocko
@ 2020-01-25 13:26 ` Pingfan Liu
0 siblings, 0 replies; 7+ messages in thread
From: Pingfan Liu @ 2020-01-25 13:26 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, Linux-MM, David Hildenbrand, Dan Williams,
Oscar Salvador, Baoquan He, Qian Cai, Kexec Mailing List,
Kazuhito Hagio
On Fri, Jan 24, 2020 at 2:49 PM Michal Hocko <mhocko@suse.com> wrote:
>
> On Thu 23-01-20 19:10:47, Andrew Morton wrote:
> > On Mon, 20 Jan 2020 08:29:39 +0100 Michal Hocko <mhocko@suse.com> wrote:
> >
> > > On Mon 20-01-20 10:33:14, Pingfan Liu wrote:
> > > > After commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug"),
> > > > when a mem section is fully deactivated, section_mem_map still records the
> > > > section's start pfn, which is not used any more and will be reassigned
> > > > during re-added.
> > > >
> > > > In analogy with alloc/free pattern, it is better to clear all fields of
> > > > section_mem_map.
> > > >
> > > > Beside this, it breaks the user space tool "makedumpfile" [1], which makes
> > > > assumption that a hot-removed section has mem_map as NULL, instead of
> > > > checking directly against SECTION_MARKED_PRESENT bit. (makedumpfile will be
> > > > better to change the assumption, and need a patch)
> > > >
> > > > The bug can be reproduced on IBM POWERVM by "drmgr -c mem -r -q 5" ,
> > > > trigger a crash, and save vmcore by makedumpfile
> > >
> > > While makedumpfile lives very closely to the kernel and occasional
> > > breakage is to be expected I still believe that Fixes: ba72b4c8cf60
> > > is due.
> >
> > But not a cc:stable?
>
> Well, I wouldn't say this is really critical. makedumpfile will get its
> fix... But if people think it would be useful in stable I won't oppose.
Yes, I think this patch is no more than a prototype improvement, and
makedumpfile has better to get its fix.
And I have sent a patch to kexec-list for it.
(http://lists.infradead.org/pipermail/kexec/2020-January/024406.html)
Thanks,
Pingfan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-25 13:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-20 2:33 [PATCHv2] mm/sparse: reset section's mem_map when fully deactivated Pingfan Liu
2020-01-20 7:29 ` Michal Hocko
2020-01-20 9:03 ` David Hildenbrand
2020-01-20 10:12 ` Pingfan Liu
2020-01-24 3:10 ` Andrew Morton
2020-01-24 6:49 ` Michal Hocko
2020-01-25 13:26 ` Pingfan Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox