lkml.org 
[lkml]   [2012]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH -V5 06/14] hugetlb: Simplify migrate_huge_page
Date
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> writes:

> (2012/04/07 3:50), Aneesh Kumar K.V wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> Since we migrate only one hugepage don't use linked list for passing
>> the page around. Directly pass page that need to be migrated as argument.
>> This also remove the usage page->lru in migrate path.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
>
> seems good to me. I have one question below.
>
>
>> ---

...... snip ......


>> - list_add(&hpage->lru, &pagelist);
>> - ret = migrate_huge_pages(&pagelist, new_page, MPOL_MF_MOVE_ALL, 0,
>> - true);
>> + ret = migrate_huge_page(page, new_page, MPOL_MF_MOVE_ALL, 0, true);
>> + put_page(page);
>> if (ret) {
>> - struct page *page1, *page2;
>> - list_for_each_entry_safe(page1, page2, &pagelist, lru)
>> - put_page(page1);
>> -
>> pr_info("soft offline: %#lx: migration failed %d, type %lx\n",
>> pfn, ret, page->flags);
>> - if (ret > 0)
>> - ret = -EIO; <---------------------------- here
>> return ret;
>> }
>> done:
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index 51c08a0..d7eb82d 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -929,15 +929,8 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
>> if (anon_vma)
>> put_anon_vma(anon_vma);
>> unlock_page(hpage);

.... snip .....

>> -
>> - rc = unmap_and_move_huge_page(get_new_page,
>> - private, page, pass > 2, offlining,
>> - mode);
>> -
>> - switch(rc) {
>> - case -ENOMEM:
>> - goto out;
>> - case -EAGAIN:
>> - retry++;
>> - break;
>> - case 0:
>> - break;
>> - default:
>> - /* Permanent failure */
>> - nr_failed++;
>> - break;
>> - }
>> + break;
>> + case 0:
>> + goto out;
>> + default:
>> + rc = -EIO;
>> + goto out;
>
>
> why -EIO ? Isn't this BUG() ??

I am not sure doing a BUG() for migrate is a good idea. We may want to
return error and let the higher layer handle this. Also as you see in
the two hunks I listed above, default is mapped to -EIO in the current
code. I didn't want to change that.

-aneesh



\
 
 \ /
  Last update: 2012-04-09 10:39    [W:0.264 / U:0.364 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site