Re: [PATCH 2/3] do_migrate_range: exit loop if not_managed is true.

[Bob Liu <lliubbo@gmail.com>]

On Thu, Oct 21, 2010 at 10:25 PM, Wu Fengguang <fengguang.wu@intel.com> wrote:
> On Thu, Oct 21, 2010 at 09:28:20PM +0800, Bob Liu wrote:
>> If not_managed is true all pages will be putback to lru, so
>> break the loop earlier to skip other pages isolate.
>
> It's good fix in itself. However it's normal for isolate_lru_page() to
> fail at times (when there are active reclaimers). The failures are
> typically temporal and may well go away when offline_pages() retries
> the call. So it seems more reasonable to migrate as much as possible
> to increase the chance of complete success in next retry.
>

Hi, Wu

The original code will try to migrate pages as much as possible except
page_count(page) is true.
If page_count(page) is true, isolate more pages is mean-less, because
all of them will
be put back after the loop.

Or maybe we can skip the page_count() check?  It seems unreasonable,
if isolate one page failed and
that page was in use why it needs to put back the whole isolated list?

Thanks

>> Signed-off-by: Bob Liu <lliubbo@gmail.com>
>> ---
>>  mm/memory_hotplug.c |   10 ++++++----
>>  1 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index d4e940a..4f72184 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -709,15 +709,17 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>>                                           page_is_file_cache(page));
>>
>>               } else {
>> -                     /* Becasue we don't have big zone->lock. we should
>> -                        check this again here. */
>> -                     if (page_count(page))
>> -                             not_managed++;
>>  #ifdef CONFIG_DEBUG_VM
>>                       printk(KERN_ALERT "removing pfn %lx from LRU failed\n",
>>                              pfn);
>>                       dump_page(page);
>>  #endif
>> +                     /* Becasue we don't have big zone->lock. we should
>> +                        check this again here. */
>> +                     if (page_count(page)) {
>> +                             not_managed++;
>> +                             break;
>> +                     }
>>               }
>>       }
>>       ret = -EBUSY;
>> --
>> 1.5.6.3
>
-- 
Regards,
--Bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>