Messages in this thread | | | From | SeongJae Park <> | Subject | Re: [PATCH v2 1/2] mm/memory_hotplug: return nothing from do_migrate_range() | Date | Thu, 16 Feb 2023 16:55:12 +0000 |
| |
Hi Baolin,
On Thu, 16 Feb 2023 18:53:34 +0800 Baolin Wang <baolin.wang@linux.alibaba.com> wrote:
> > > On 2/16/2023 7:02 AM, SeongJae Park wrote: > > Return value mechanism of do_migrate_range() is not very simple, while > > no caller of the function checks the return value. Make the function > > return nothing to be more simple. > > > > Suggested-by: David Hildenbrand <david@redhat.com> > > Signed-off-by: SeongJae Park <sj@kernel.org> > > --- > > mm/memory_hotplug.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > > index bcb0dc41c2f2..6c615ba1a5c7 100644 > > --- a/mm/memory_hotplug.c > > +++ b/mm/memory_hotplug.c > > @@ -1620,8 +1620,7 @@ static int scan_movable_pages(unsigned long start, unsigned long end, > > return 0; > > } > > > > -static int > > -do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) > > +static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) > > { > > unsigned long pfn; > > struct page *page, *head; > > @@ -1721,8 +1720,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) > > putback_movable_pages(&source); > > } > > } > > - > > - return ret; > > Nit: while we are at it, can we also remove the "TODO" comment in > offline_pages()? > > ret = scan_movable_pages(pfn, end_pfn, &pfn); > if (!ret) { > /* > * TODO: fatal migration failures should bail > * out > */ > do_migrate_range(pfn, end_pfn); > } > > With David's comments: > Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
As David mentioned we're just still unclear what issues are fatal[1], I was thinking we didn't forgive the future work, so left the comment. If anyone has different opinion or I'm getting something wrong, please let me know.
[1] https://lore.kernel.org/linux-mm/388b9a93-423f-33f8-0495-2a4a290fd1aa@redhat.com/
Thanks, SJ
| |