Re: [BUGFIX][PATCH] fix wrong lru rotate back at lumpty reclaim
From: Minchan Kim
Date: Tue Jun 09 2009 - 08:07:52 EST
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) !=
> Â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 ?
>> 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. :)
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/