Subject: Re: [PATCH mmotm 3/3] mm, compaction: prevent nr_isolated_* from
 going negative
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
References: <1461591269-28615-1-git-send-email-vbabka@suse.cz>
 <1461591350-28700-1-git-send-email-vbabka@suse.cz>
 <1461591350-28700-4-git-send-email-vbabka@suse.cz>
 <20160426005503.GC2707@js1304-P5Q-DELUXE>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, Michal Hocko <mhocko@suse.com>,
        Hugh Dickins <hughd@google.com>, Rik van Riel <riel@redhat.com>
From: Vlastimil Babka <vbabka@suse.cz>
Message-ID: <571FC43D.6010102@suse.cz>
Date: Tue, 26 Apr 2016 21:40:45 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.1
MIME-Version: 1.0
In-Reply-To: <20160426005503.GC2707@js1304-P5Q-DELUXE>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1494
Lines: 35

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.

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

Vlastimil

> Thanks.
>