From patchwork Thu Jul 21 14:53:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Hocko X-Patchwork-Id: 9241835 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3DA43607D3 for ; Thu, 21 Jul 2016 14:56:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2D8A927DE0 for ; Thu, 21 Jul 2016 14:56:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 21E9727EE2; Thu, 21 Jul 2016 14:56:56 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from mx3-phx2.redhat.com (mx3-phx2.redhat.com [209.132.183.24]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 5EA8E27DE0 for ; Thu, 21 Jul 2016 14:56:55 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx3-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u6LErHB3008536; Thu, 21 Jul 2016 10:53:17 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u6LErFFE010902 for ; Thu, 21 Jul 2016 10:53:15 -0400 Received: from mx1.redhat.com (ext-mx06.extmail.prod.ext.phx2.redhat.com [10.5.110.30]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u6LErFir024110 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 21 Jul 2016 10:53:15 -0400 Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E7E073B725; Thu, 21 Jul 2016 14:53:13 +0000 (UTC) Received: by mail-wm0-f68.google.com with SMTP id q128so2723289wma.1; Thu, 21 Jul 2016 07:53:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=UTTG2aL1sNcPgxYhEfXL+MYZuNViQQlIoXKXnrKYOsc=; b=U3dK14WP99CFk1wLBr0n1PLX5Oi7SLapEJn22R/dxAvbjsDsZR1kC64NgaymxjlGco iadNiI4Rv+Eyg54fMQ37ettJXStMzQHSZc7GBF9irOYtI/v+xiotr2Pb2+5gieHryXJt ij6/B5EZ0EB5WSFPf4emeo0ePSs7VQU9/RwzdoRoKCc3LSnhIADkV+S+GmV9iNS+t9VL /0iatpjzZMhnfX+mMx8zylPq6sOm6U3nTpuo5TOX1LZJmSd4RlquFvUGotuRONyMErbv sw3wOsIKLvR3U/GVIWb3sIh64yvMVCAeTASqVJhwpW2FL4qgYT/NFqF+NqtFVcH23w/u JH0w== X-Gm-Message-State: ALyK8tK9xk861eCKs07ljAMMNdB5DKOeF4iB0mCFlTarejy38/px5/cz4VD0VPfOcsG3UQ== X-Received: by 10.28.19.134 with SMTP id 128mr17854936wmt.40.1469112791897; Thu, 21 Jul 2016 07:53:11 -0700 (PDT) Received: from localhost ([80.188.202.66]) by smtp.gmail.com with ESMTPSA id m127sm4619347wmm.21.2016.07.21.07.53.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 21 Jul 2016 07:53:10 -0700 (PDT) Date: Thu, 21 Jul 2016 16:53:10 +0200 From: Michal Hocko To: Johannes Weiner Message-ID: <20160721145309.GR26379@dhcp22.suse.cz> References: <1468831164-26621-1-git-send-email-mhocko@kernel.org> <1468831285-27242-1-git-send-email-mhocko@kernel.org> <20160719135426.GA31229@cmpxchg.org> <20160720081541.GF11249@dhcp22.suse.cz> <20160721085202.GC26379@dhcp22.suse.cz> <20160721121300.GA21806@cmpxchg.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160721121300.GA21806@cmpxchg.org> User-Agent: Mutt/1.6.0 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 21 Jul 2016 14:53:14 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 21 Jul 2016 14:53:14 +0000 (UTC) for IP:'74.125.82.68' DOMAIN:'mail-wm0-f68.google.com' HELO:'mail-wm0-f68.google.com' FROM:'mstsxfx@gmail.com' RCPT:'' X-RedHat-Spam-Score: 1.082 * (BAYES_50, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, HEXHASH_WORD, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_PASS) 74.125.82.68 mail-wm0-f68.google.com 74.125.82.68 mail-wm0-f68.google.com X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.30 X-loop: dm-devel@redhat.com Cc: Tetsuo Handa , LKML , linux-mm@kvack.org, dm-devel@redhat.com, Mikulas Patocka , Mel Gorman , David Rientjes , Ondrej Kozina , Andrew Morton Subject: Re: [dm-devel] [RFC PATCH 1/2] mempool: do not consume memory reserves from the reclaim path X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Virus-Scanned: ClamAV using ClamSMTP On Thu 21-07-16 08:13:00, Johannes Weiner wrote: > On Thu, Jul 21, 2016 at 10:52:03AM +0200, Michal Hocko wrote: > > Look, there are > > $ git grep mempool_alloc | wc -l > > 304 > > > > many users of this API and we do not want to flip the default behavior > > which is there for more than 10 years. So far you have been arguing > > about potential deadlocks and haven't shown any particular path which > > would have a direct or indirect dependency between mempool and normal > > allocator and it wouldn't be a bug. As the matter of fact the change > > we are discussing here causes a regression. If you want to change the > > semantic of mempool allocator then you are absolutely free to do so. In > > a separate patch which would be discussed with IO people and other > > users, though. But we _absolutely_ want to fix the regression first > > and have a simple fix for 4.6 and 4.7 backports. At this moment there > > are revert and patch 1 on the table. The later one should make your > > backtrace happy and should be only as a temporal fix until we find out > > what is actually misbehaving on your systems. If you are not interested > > to pursue that way I will simply go with the revert. > > +1 > > It's very unlikely that decade-old mempool semantics are suddenly a > fundamental livelock problem, when all the evidence we have is one > hang and vague speculation. Given that the patch causes regressions, > and that the bug is most likely elsewhere anyway, a full revert rather > than merely-less-invasive mempool changes makes the most sense to me. OK, fair enough. What do you think about the following then? Mikulas, I have dropped your Tested-by and Reviewed-by because the patch is different but unless you have hit the OOM killer then the testing results should be same. Reviewed-by: Johannes Weiner Acked-by: NeilBrown --- >From d64815758c212643cc1750774e2751721685059a Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Thu, 21 Jul 2016 16:40:59 +0200 Subject: [PATCH] Revert "mm, mempool: only set __GFP_NOMEMALLOC if there are free elements" This reverts commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. There has been a report about OOM killer invoked when swapping out to a dm-crypt device. The primary reason seems to be that the swapout out IO managed to completely deplete memory reserves. Ondrej was able to bisect and explained the issue by pointing to f9054c70d28b ("mm, mempool: only set __GFP_NOMEMALLOC if there are free elements"). The reason is that the swapout path is not throttled properly because the md-raid layer needs to allocate from the generic_make_request path which means it allocates from the PF_MEMALLOC context. dm layer uses mempool_alloc in order to guarantee a forward progress which used to inhibit access to memory reserves when using page allocator. This has changed by f9054c70d28b ("mm, mempool: only set __GFP_NOMEMALLOC if there are free elements") which has dropped the __GFP_NOMEMALLOC protection when the memory pool is depleted. If we are running out of memory and the only way forward to free memory is to perform swapout we just keep consuming memory reserves rather than throttling the mempool allocations and allowing the pending IO to complete up to a moment when the memory is depleted completely and there is no way forward but invoking the OOM killer. This is less than optimal. The original intention of f9054c70d28b was to help with the OOM situations where the oom victim depends on mempool allocation to make a forward progress. David has mentioned the following backtrace: schedule schedule_timeout io_schedule_timeout mempool_alloc __split_and_process_bio dm_request generic_make_request submit_bio mpage_readpages ext4_readpages __do_page_cache_readahead ra_submit filemap_fault handle_mm_fault __do_page_fault do_page_fault page_fault We do not know more about why the mempool is depleted without being replenished in time, though. In any case the dm layer shouldn't depend on any allocations outside of the dedicated pools so a forward progress should be guaranteed. If this is not the case then the dm should be fixed rather than papering over the problem and postponing it to later by accessing more memory reserves. mempools are a mechanism to maintain dedicated memory reserves to guaratee forward progress. Allowing them an unbounded access to the page allocator memory reserves is going against the whole purpose of this mechanism. Bisected-by: Ondrej Kozina Signed-off-by: Michal Hocko --- mm/mempool.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/mm/mempool.c b/mm/mempool.c index 8f65464da5de..5ba6c8b3b814 100644 --- a/mm/mempool.c +++ b/mm/mempool.c @@ -306,36 +306,25 @@ EXPORT_SYMBOL(mempool_resize); * returns NULL. Note that due to preallocation, this function * *never* fails when called from process contexts. (it might * fail if called from an IRQ context.) - * Note: neither __GFP_NOMEMALLOC nor __GFP_ZERO are supported. + * Note: using __GFP_ZERO is not supported. */ -void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask) +void * mempool_alloc(mempool_t *pool, gfp_t gfp_mask) { void *element; unsigned long flags; wait_queue_t wait; gfp_t gfp_temp; - /* If oom killed, memory reserves are essential to prevent livelock */ - VM_WARN_ON_ONCE(gfp_mask & __GFP_NOMEMALLOC); - /* No element size to zero on allocation */ VM_WARN_ON_ONCE(gfp_mask & __GFP_ZERO); - might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM); + gfp_mask |= __GFP_NOMEMALLOC; /* don't allocate emergency reserves */ gfp_mask |= __GFP_NORETRY; /* don't loop in __alloc_pages */ gfp_mask |= __GFP_NOWARN; /* failures are OK */ gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO); repeat_alloc: - if (likely(pool->curr_nr)) { - /* - * Don't allocate from emergency reserves if there are - * elements available. This check is racy, but it will - * be rechecked each loop. - */ - gfp_temp |= __GFP_NOMEMALLOC; - } element = pool->alloc(gfp_temp, pool->pool_data); if (likely(element != NULL)) @@ -359,12 +348,11 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask) * We use gfp mask w/o direct reclaim or IO for the first round. If * alloc failed with that and @pool was empty, retry immediately. */ - if ((gfp_temp & ~__GFP_NOMEMALLOC) != gfp_mask) { + if (gfp_temp != gfp_mask) { spin_unlock_irqrestore(&pool->lock, flags); gfp_temp = gfp_mask; goto repeat_alloc; } - gfp_temp = gfp_mask; /* We must not sleep if !__GFP_DIRECT_RECLAIM */ if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {