linux-kernel - Re: [PATCH mmotm 3/3] mm, compaction: prevent nr_isolated

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20160427005927.GC6336@js1304-P5Q-DELUXE>
Date:	Wed, 27 Apr 2016 09:59:28 +0900
From:	Joonsoo Kim <iamjoonsoo.kim@....com>
To:	Vlastimil Babka <vbabka@...e.cz>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Michal Hocko <mhocko@...e.com>,
	Hugh Dickins <hughd@...gle.com>, Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH mmotm 3/3] mm, compaction: prevent nr_isolated_* from
 going negative

On Tue, Apr 26, 2016 at 09:40:45PM +0200, Vlastimil Babka wrote:
> On 04/26/2016 02:55 AM, Joonsoo Kim wrote:
> >On Mon, Apr 25, 2016 at 03:35:50PM +0200, Vlastimil Babka wrote:
> >>@@ -846,9 +845,11 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> >> 				spin_unlock_irqrestore(&zone->lru_lock,	flags);
> >> 				locked = false;
> >> 			}
> >>-			putback_movable_pages(migratelist);
> >>-			nr_isolated = 0;
> >>+			acct_isolated(zone, cc);
> >>+			putback_movable_pages(&cc->migratepages);
> >>+			cc->nr_migratepages = 0;
> >> 			cc->last_migrated_pfn = 0;
> >>+			nr_isolated = 0;
> >
> >Is it better to use separate list and merge it cc->migratepages when
> >finishing instead of using cc->migratepages directly? If
> >isolate_migratepages() try to isolate more than one page block and keep
> >isolated page on previous pageblock, this putback all will invalidate
> >all the previous work. It would be beyond of the scope of this
> >function. Now, isolate_migratepages() try to isolate the page in one
> >pageblock so this code is safe. But, I think that removing such
> >dependency will be helpful in the future. I'm not strongly insisting it
> >so if you think it's not useful thing, please ignore this comment.
> 
> migratelist was merely a reference to cc->migratepages, so it
> wouldn't prevent the situation you are suggesting. A truly separate
> list would need to be appended to cc->migratepages when leaving
> isolate_migratepages_block() and there's no need to do that right
> now.

What I suggest is using truly separate list by defining LIST_HEAD(xxx)
on top of the function. But, I'm okay you think that there's no need
to do it right now.

> 
> BTW, can you check patch 1/3? Thanks!

Done.

Thanks.