| Message ID | 20220409073846.22286-4-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few cleanup and fixup patches for migration | expand |
On Sat, Apr 09, 2022 at 03:38:45PM +0800, Miaohe Lin wrote: > We might fail to isolate huge page due to e.g. the page is under migration > which cleared HPageMigratable. So we should return -EBUSY in this case > rather than always return 1 which could confuse the user. > > Fixes: e8db67eb0ded ("mm: migrate: move_pages() supports thp migration") > Reviewed-by: Muchun Song <songmuchun@bytedance.com> > Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/migrate.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 381963231a62..044656a14ae2 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1632,10 +1632,8 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr, > goto out_putpage; > > if (PageHuge(page)) { > - if (PageHead(page)) { > - isolate_huge_page(page, pagelist); > - err = 1; > - } > + if (PageHead(page)) > + err = isolate_huge_page(page, pagelist) ? 1 : -EBUSY; I think a: err = isolate_huge_page(page, pagelist); if (!err) err = 1; would be a lot more readable here.
On 2022/4/11 22:10, Christoph Hellwig wrote: > On Sat, Apr 09, 2022 at 03:38:45PM +0800, Miaohe Lin wrote: >> We might fail to isolate huge page due to e.g. the page is under migration >> which cleared HPageMigratable. So we should return -EBUSY in this case >> rather than always return 1 which could confuse the user. >> >> Fixes: e8db67eb0ded ("mm: migrate: move_pages() supports thp migration") >> Reviewed-by: Muchun Song <songmuchun@bytedance.com> >> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/migrate.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 381963231a62..044656a14ae2 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1632,10 +1632,8 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr, >> goto out_putpage; >> >> if (PageHuge(page)) { >> - if (PageHead(page)) { >> - isolate_huge_page(page, pagelist); >> - err = 1; >> - } >> + if (PageHead(page)) >> + err = isolate_huge_page(page, pagelist) ? 1 : -EBUSY; > > I think a: > > err = isolate_huge_page(page, pagelist); > if (!err) > err = 1; Many thanks for your comment. IIUC, isolate_huge_page does not return the wanted error code. So the above code won't do the right thing. Thanks. > > would be a lot more readable here. > > . >
diff --git a/mm/migrate.c b/mm/migrate.c index 381963231a62..044656a14ae2 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1632,10 +1632,8 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr, goto out_putpage; if (PageHuge(page)) { - if (PageHead(page)) { - isolate_huge_page(page, pagelist); - err = 1; - } + if (PageHead(page)) + err = isolate_huge_page(page, pagelist) ? 1 : -EBUSY; } else { struct page *head;
We might fail to isolate huge page due to e.g. the page is under migration which cleared HPageMigratable. So we should return -EBUSY in this case rather than always return 1 which could confuse the user. Fixes: e8db67eb0ded ("mm: migrate: move_pages() supports thp migration") Reviewed-by: Muchun Song <songmuchun@bytedance.com> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/migrate.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)