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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <28c262360906090507u75f5b594o71906777a91efa1@mail.gmail.com>
Date:	Tue, 9 Jun 2009 21:07:16 +0900
From:	Minchan Kim <minchan.kim@...il.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Andy Whitcroft <apw@...onical.com>
Cc:	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"kosaki.motohiro@...fujitsu.com" <kosaki.motohiro@...fujitsu.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	riel@...hat.com
Subject: Re: [BUGFIX][PATCH] fix wrong lru rotate back at lumpty reclaim

2009/6/9 KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>:
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/