Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932919AbdCISLm (ORCPT ); Thu, 9 Mar 2017 13:11:42 -0500 Received: from gum.cmpxchg.org ([85.214.110.215]:39524 "EHLO gum.cmpxchg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752627AbdCISLl (ORCPT ); Thu, 9 Mar 2017 13:11:41 -0500 Date: Thu, 9 Mar 2017 13:05:40 -0500 From: Johannes Weiner To: Rik van Riel Cc: Michal Hocko , Andrew Morton , Mel Gorman , Vlastimil Babka , Tetsuo Handa , linux-mm@kvack.org, LKML , Michal Hocko Subject: Re: [PATCH] mm, vmscan: do not loop on too_many_isolated for ever Message-ID: <20170309180540.GA8678@cmpxchg.org> References: <20170307133057.26182-1-mhocko@kernel.org> <1488916356.6405.4.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1488916356.6405.4.camel@redhat.com> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1932 Lines: 42 On Tue, Mar 07, 2017 at 02:52:36PM -0500, Rik van Riel wrote: > It only does this to some extent. ?If reclaim made > no progress, for example due to immediately bailing > out because the number of already isolated pages is > too high (due to many parallel reclaimers), the code > could hit the "no_progress_loops > MAX_RECLAIM_RETRIES" > test without ever looking at the number of reclaimable > pages. Hm, there is no early return there, actually. We bump the loop counter every time it happens, but then *do* look at the reclaimable pages. > Could that create problems if we have many concurrent > reclaimers? With increased concurrency, the likelihood of OOM will go up if we remove the unlimited wait for isolated pages, that much is true. I'm not sure that's a bad thing, however, because we want the OOM killer to be predictable and timely. So a reasonable wait time in between 0 and forever before an allocating thread gives up under extreme concurrency makes sense to me. > It may be OK, I just do not understand all the implications. > > I like the general direction your patch takes the code in, > but I would like to understand it better... I feel the same way. The throttling logic doesn't seem to be very well thought out at the moment, making it hard to reason about what happens in certain scenarios. In that sense, this patch isn't really an overall improvement to the way things work. It patches a hole that seems to be exploitable only from an artificial OOM torture test, at the risk of regressing high concurrency workloads that may or may not be artificial. Unless I'm mistaken, there doesn't seem to be a whole lot of urgency behind this patch. Can we think about a general model to deal with allocation concurrency? Unlimited parallel direct reclaim is kinda bonkers in the first place. How about checking for excessive isolation counts from the page allocator and putting allocations on a waitqueue?