From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932378AbcJLIcx (ORCPT ); Wed, 12 Oct 2016 04:32:53 -0400 Received: from mail-wm0-f49.google.com ([74.125.82.49]:38491 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932088AbcJLIcn (ORCPT ); Wed, 12 Oct 2016 04:32:43 -0400 Date: Wed, 12 Oct 2016 09:33:28 +0200 From: Michal Hocko To: Minchan Kim Cc: Andrew Morton , Mel Gorman , Vlastimil Babka , Joonsoo Kim , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Sangseok Lee Subject: Re: [PATCH v2 4/4] mm: make unreserve highatomic functions reliable Message-ID: <20161012073328.GC9504@dhcp22.suse.cz> References: <1476250416-22733-1-git-send-email-minchan@kernel.org> <1476250416-22733-5-git-send-email-minchan@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1476250416-22733-5-git-send-email-minchan@kernel.org> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 12-10-16 14:33:36, Minchan Kim wrote: [...] > @@ -2138,8 +2146,10 @@ static bool unreserve_highatomic_pageblock(const struct alloc_context *ac) > */ > set_pageblock_migratetype(page, ac->migratetype); > ret = move_freepages_block(zone, page, ac->migratetype); > - spin_unlock_irqrestore(&zone->lock, flags); > - return ret; > + if (!drain && ret) { > + spin_unlock_irqrestore(&zone->lock, flags); > + return ret; > + } I've already mentioned that during the previous discussion. This sounds overly aggressive to me. Why do we want to drain the whole reserve and risk that we won't be able to build up a new one after OOM. Doing one block at the time should be sufficient IMHO. if (ret) { spin_unlock_irqrestore(&zone->lock, flags); return ret; } will do the trick and work both for drain and !drain cases which is a good thing. Because even !drain case would like to see a block freed. The only difference between those two is that the drain one would really like to free something and ignore the "at least one block" reserve. -- Michal Hocko SUSE Labs