| Message ID | 20220418141253.24298-10-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few cleanup and fixup patches for compaction | expand |
Please cc David H on memhotplug stuff. On Mon, 18 Apr 2022 22:12:50 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote: > It's possible that kcompactd_run could fail to run kcompactd for a hot > added node and leave pgdat->kcompactd as NULL. So pgdat->kcompactd should > be checked here to avoid possible NULL pointer dereference. > > .. > > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -3052,7 +3052,8 @@ static int kcompactd_cpu_online(unsigned int cpu) > > if (cpumask_any_and(cpu_online_mask, mask) < nr_cpu_ids) > /* One of our CPUs online: restore mask */ > - set_cpus_allowed_ptr(pgdat->kcompactd, mask); > + if (pgdat->kcompactd) > + set_cpus_allowed_ptr(pgdat->kcompactd, mask); > } > return 0; > } Why not fail to bring the node online if kcompactd_run() failed? Also, should we panic the system if kcompactd_run() failed in kcompactd_init()?
On 2022/4/19 11:55, Andrew Morton wrote: > Please cc David H on memhotplug stuff. I'm relying on the scripts/get_maintainer.pl to take me the right people now. Will take care of it. Many thanks for your remind! > > On Mon, 18 Apr 2022 22:12:50 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote: > >> It's possible that kcompactd_run could fail to run kcompactd for a hot >> added node and leave pgdat->kcompactd as NULL. So pgdat->kcompactd should >> be checked here to avoid possible NULL pointer dereference. >> >> .. >> >> --- a/mm/compaction.c >> +++ b/mm/compaction.c >> @@ -3052,7 +3052,8 @@ static int kcompactd_cpu_online(unsigned int cpu) >> >> if (cpumask_any_and(cpu_online_mask, mask) < nr_cpu_ids) >> /* One of our CPUs online: restore mask */ >> - set_cpus_allowed_ptr(pgdat->kcompactd, mask); >> + if (pgdat->kcompactd) >> + set_cpus_allowed_ptr(pgdat->kcompactd, mask); >> } >> return 0; >> } > > Why not fail to bring the node online if kcompactd_run() failed? kcompactd_run() is allowed to fail since it's introduced via commit 698b1b30642 ("mm, compaction: introduce kcompactd"). > > Also, should we panic the system if kcompactd_run() failed in > kcompactd_init()? So I think this might not be a critical issue. We could live with it anyway. And in fact, CONFIG_COMPACTION is even not enabled in some systems. Thanks! > > . >
On 19.04.22 08:47, Miaohe Lin wrote: > On 2022/4/19 11:55, Andrew Morton wrote: >> Please cc David H on memhotplug stuff. > > I'm relying on the scripts/get_maintainer.pl to take me the right people now. > Will take care of it. Many thanks for your remind! Yes, we should add add a memhotplug in MAINTAINERS subsections for good so I can punch in my name. Let me add that to my TODO list.
On 2022/4/19 15:42, David Hildenbrand wrote: > On 19.04.22 08:47, Miaohe Lin wrote: >> On 2022/4/19 11:55, Andrew Morton wrote: >>> Please cc David H on memhotplug stuff. >> >> I'm relying on the scripts/get_maintainer.pl to take me the right people now. >> Will take care of it. Many thanks for your remind! > > Yes, we should add add a memhotplug in MAINTAINERS subsections for good > so I can punch in my name. Let me add that to my TODO list. > That will be really convenient. Thanks!
diff --git a/mm/compaction.c b/mm/compaction.c index 562f274b2c51..82c54d70a978 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -3052,7 +3052,8 @@ static int kcompactd_cpu_online(unsigned int cpu) if (cpumask_any_and(cpu_online_mask, mask) < nr_cpu_ids) /* One of our CPUs online: restore mask */ - set_cpus_allowed_ptr(pgdat->kcompactd, mask); + if (pgdat->kcompactd) + set_cpus_allowed_ptr(pgdat->kcompactd, mask); } return 0; }
It's possible that kcompactd_run could fail to run kcompactd for a hot added node and leave pgdat->kcompactd as NULL. So pgdat->kcompactd should be checked here to avoid possible NULL pointer dereference. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/compaction.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)