Re: [BUGFIX][PATCH] fix wrong lru rotate back at lumpty reclaim

[Minchan Kim]

2009/6/9 KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>:
> Minchan Kim wrote:
>
>> I mean follow as
>> Â908 Â Â Â Â /*
>> Â909 Â Â Â Â Â* Attempt to take all pages in the order aligned region
>> Â910 Â Â Â Â Â* surrounding the tag page. ÂOnly take those pages of
>> Â911 Â Â Â Â Â* the same active state as that tag page. ÂWe may safely
>> Â912 Â Â Â Â Â* round the target page pfn down to the requested order
>> Â913 Â Â Â Â Â* as the mem_map is guarenteed valid out to MAX_ORDER,
>> Â914 Â Â Â Â Â* where that page is in a different zone we will detect
>> Â915 Â Â Â Â Â* it from its zone id and abort this block scan.
>> Â916 Â Â Â Â Â*/
>> Â917 Â Â Â Â zone_id = page_zone_id(page);
>>
> But what this code really do is.
> ==
> 931 Â Â Â Â Â Â Â Â Â Â Â Â /* Check that we have not crossed a zone
> boundary. */
> Â932 Â Â Â Â Â Â Â Â Â Â Â Â if (unlikely(page_zone_id(cursor_page) !=
> zone_id))
> Â933 Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â continue;
> ==
> continue. I think this should be "break"
> I wonder what "This block scan" means is "scanning this aligned block".

It is to find first page in same zone with target page when we have
crossed a zone.
so it shouldn't stop due to that.

I think 'abort' means stopping only the page.
If it is right, it would be better to change follow as.
"and continue scanning next page"

Let's Cced Andy Whitcroft.

> But I think the whoe code is not written as commented.
>
>>
>>>> If I understand it properly , don't we add goto phrase ?
>>>>
>>> No.
>>
>> If it is so, the break also is meaningless.
>>
> yes. I'll remove it. But need to add "exit from for loop" logic again.
>
> I'm sorry that the wrong logic of this loop was out of my sight.
> I'll review and rewrite this part all, tomorrow.

Yes. I will review tomorrow, too. :)

> Thanks,
> -Kame
>
>



-- 
Kinds regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/