From patchwork Fri Jun 1 08:04:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Hocko X-Patchwork-Id: 10442641 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 BCF9A602BD for ; Fri, 1 Jun 2018 08:04:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AC36728E95 for ; Fri, 1 Jun 2018 08:04:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9F19528F48; Fri, 1 Jun 2018 08:04:51 +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=-2.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 069EB28E95 for ; Fri, 1 Jun 2018 08:04:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E0FC26B0005; Fri, 1 Jun 2018 04:04:49 -0400 (EDT) Delivered-To: linux-mm-outgoing@kvack.org Received: by kanga.kvack.org (Postfix, from userid 40) id DC0686B0006; Fri, 1 Jun 2018 04:04:49 -0400 (EDT) X-Original-To: int-list-linux-mm@kvack.org X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CB0106B0007; Fri, 1 Jun 2018 04:04:49 -0400 (EDT) X-Original-To: linux-mm@kvack.org X-Delivered-To: linux-mm@kvack.org Received: from mail-pl0-f69.google.com (mail-pl0-f69.google.com [209.85.160.69]) by kanga.kvack.org (Postfix) with ESMTP id 8679D6B0005 for ; Fri, 1 Jun 2018 04:04:49 -0400 (EDT) Received: by mail-pl0-f69.google.com with SMTP id i1-v6so14856374pld.11 for ; Fri, 01 Jun 2018 01:04:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-original-authentication-results:x-gm-message-state:date:from:to :cc:subject:message-id:references:mime-version:content-disposition :in-reply-to:user-agent; bh=cL4/KYIyraDqLL+mKi5LXiK04/X+muGE0bocfvYwntA=; b=AU0eTgflXT3n3gGyjMFKjnJ21gz49lVYHCOU1whPJpnEsOCgzRD+RFPRd+kBZ3X6VE +hO8G2t6qgdKiQTw88LGztv+v+FSCPLlZMldG59rMU3A/sD5Zc2+cbCIeDc3rV4SOPL0 2s/1Yy0YM5UCmG71e4wKzLnwBlRBbPOg0MQeUMBCBMrg0hRy3eAeuc2rjxmg5J6D6Boi yWTZDCZkoKu7QCw14KwvLtdC2bsgNSAM+37ksbv05/VDoeu+gQl7Z77ZoTBx/gfNKaYT QwLNC1ey53K8uOebZyHC5V6hBS+Al9ZTB9OwatJlwHMGQ0GfZV22aat9cSHWZYr8mrp3 g7Ug== X-Original-Authentication-Results: mx.google.com; spf=softfail (google.com: domain of transitioning mhocko@kernel.org does not designate 195.135.220.15 as permitted sender) smtp.mailfrom=mhocko@kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org X-Gm-Message-State: ALKqPwdgUxL4iAeL6Ovlh1RRJoOz2YS10SIvm3V1ETbeJo5S0gyvHYiU B4Unwa+LITnBqdBQC6it9QjuxTh0pnbOqRDS/j2dJm3Im02IyQlOzrskVhqXqlnQEMX/EW1LnfW VS0pJP27kgDhAd0iEkrkLALoAud86y2+fAvni+qyWG6uMt/dv6jWiS5wF7IcY5g8= X-Received: by 2002:a17:902:3281:: with SMTP id z1-v6mr9997430plb.226.1527840289203; Fri, 01 Jun 2018 01:04:49 -0700 (PDT) X-Google-Smtp-Source: ADUXVKIPs9fCS6Gn5TLfZXbmEiCLSecZ5LJSGTBKkmNzcdh+iRljCNEurg1MA9R8m8dnhLeRDXhY X-Received: by 2002:a17:902:3281:: with SMTP id z1-v6mr9997386plb.226.1527840288304; Fri, 01 Jun 2018 01:04:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1527840288; cv=none; d=google.com; s=arc-20160816; b=J5q7onrWqBybt1IkwwDivT4U0KpStxkje7Q5XVxk0rrI3lWiGwJa7ykfPfODTdDTsu wN6Slz5wy7o0khaf0ENXHXsvMNhFFVAPosyOpCmN/0hE1jEbD0K3lchpbb5TZx3ywNnB OUeZN4svwcwGx/+FF+ljq6O0vWyzIb/16w5Ht+ZrkhonqsdL7ox3/5Vux5TneY0hWn4a exyVjt1GMLjLmFCaQW+gObBzirBeWB1fzt6WRE+4rHxogYUnyhdtx0ndLO6eRdpQBnsn Me1V7bagqKwL8OQ0oekb46tIlBxly9vuqH2q0UCbqTIO7UvedvyzyiaLKkJ/6Kz32+D8 9bNw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:arc-authentication-results; bh=cL4/KYIyraDqLL+mKi5LXiK04/X+muGE0bocfvYwntA=; b=UsRmtzc6t/dYjZye9mJzSPzShLqtABpnNNblWYxyNHDZdu6oajdnVNsgIzODd5Kx/B n0F2Z9dmStgXpGrT/hJn/I051AWHZxwuFiF3zmR5bidyMktG06dHSvitYxPJ75/O5Ry/ Qs6rcJoGsOrHWIMPnWu1PNmvlJ23zsvIjXwQ/80u7J4Jd0HW3NqqWUgGvbif1WsXH0bv Kv68Bx6TUGd1ylS/JLHi8BID6Di8DrJVSWQhQL2tOHJWGsFUiBCYAX+k/OUBxRhTIRJe fPPV/RKbl9dbCwFzq1rHVd9+Kvh0ce4wzyGMBseAwRW1Wuh75R2EtArssSqWebINGRSU qwjg== ARC-Authentication-Results: i=1; mx.google.com; spf=softfail (google.com: domain of transitioning mhocko@kernel.org does not designate 195.135.220.15 as permitted sender) smtp.mailfrom=mhocko@kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from mx2.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id v7-v6si38091975plp.304.2018.06.01.01.04.47 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 01 Jun 2018 01:04:48 -0700 (PDT) Received-SPF: softfail (google.com: domain of transitioning mhocko@kernel.org does not designate 195.135.220.15 as permitted sender) client-ip=195.135.220.15; Authentication-Results: mx.google.com; spf=softfail (google.com: domain of transitioning mhocko@kernel.org does not designate 195.135.220.15 as permitted sender) smtp.mailfrom=mhocko@kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext-too.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id DC49DADE7; Fri, 1 Jun 2018 08:04:44 +0000 (UTC) Date: Fri, 1 Jun 2018 10:04:43 +0200 From: Michal Hocko To: Tetsuo Handa Cc: Andrew Morton , torvalds@linux-foundation.org, guro@fb.com, rientjes@google.com, hannes@cmpxchg.org, vdavydov.dev@gmail.com, tj@kernel.org, linux-mm@kvack.org Subject: Re: [PATCH] mm,oom: Don't call schedule_timeout_killable() with oom_lock held. Message-ID: <20180601080443.GX15278@dhcp22.suse.cz> References: <7276d450-5e66-be56-3a17-0fc77596a3b6@i-love.sakura.ne.jp> <20180531184721.GU15278@dhcp22.suse.cz> <201806010121.w511LDbC077249@www262.sakura.ne.jp> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <201806010121.w511LDbC077249@www262.sakura.ne.jp> User-Agent: Mutt/1.9.5 (2018-04-13) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: X-Virus-Scanned: ClamAV using ClamSMTP On Fri 01-06-18 10:21:13, Tetsuo Handa wrote: > Michal Hocko wrote: > > On Fri 01-06-18 00:23:57, Tetsuo Handa wrote: > > > On 2018/05/31 19:44, Michal Hocko wrote: > > > > On Thu 31-05-18 19:10:48, Tetsuo Handa wrote: > > > >> On 2018/05/30 8:07, Andrew Morton wrote: > > > >>> On Tue, 29 May 2018 09:17:41 +0200 Michal Hocko wrote: > > > >>> > > > >>>>> I suggest applying > > > >>>>> this patch first, and then fix "mm, oom: cgroup-aware OOM killer" patch. > > > >>>> > > > >>>> Well, I hope the whole pile gets merged in the upcoming merge window > > > >>>> rather than stall even more. > > > >>> > > > >>> I'm more inclined to drop it all. David has identified significant > > > >>> shortcomings and I'm not seeing a way of addressing those shortcomings > > > >>> in a backward-compatible fashion. Therefore there is no way forward > > > >>> at present. > > > >>> > > > >> > > > >> Can we apply my patch as-is first? > > > > > > > > No. As already explained before. Sprinkling new sleeps without a strong > > > > reason is not acceptable. The issue you are seeing is pretty artificial > > > > and as such doesn're really warrant an immediate fix. We should rather > > > > go with a well thought trhough fix. In other words we should simply drop > > > > the sleep inside the oom_lock for starter unless it causes some really > > > > unexpected behavior change. > > > > > > > > > > The OOM killer did not require schedule_timeout_killable(1) to return > > > as long as the OOM victim can call __mmput(). But now the OOM killer > > > requires schedule_timeout_killable(1) to return in order to allow the > > > OOM victim to call __oom_reap_task_mm(). Thus, this is a regression. > > > > > > Artificial cannot become the reason to postpone my patch. If we don't care > > > artificialness/maliciousness, we won't need to care Spectre/Meltdown bugs. > > > > > > I'm not sprinkling new sleeps. I'm just merging existing sleeps (i.e. > > > mutex_trylock() case and !mutex_trylock() case) and updating the outdated > > > comments. > > > > Sigh. So what exactly is wrong with going simple and do > > http://lkml.kernel.org/r/20180528124313.GC27180@dhcp22.suse.cz ? > > > > Because > > (1) You are trying to apply this fix after Roman's patchset which > Andrew Morton is more inclined to drop. > > (2) You are making this fix difficult to backport because this > patch depends on Roman's patchset. > > (3) You are not fixing the bug in Roman's patchset. Sigh. Would you be more happy if this was on top of linus tree? I mean this is trivial to do so. I have provided _something_ for testing exactly as you asked for. Considering that the cgroup aware oom killer shouldn't stand in a way for global oom killer without a special configurion I do no see what is the actual problem. > (4) You are not updating the outdated comment in my patch and > Roman's patchset. But the comment is not really related to the sleep in any way. This should be a separate patch AFAICS. > (5) You are not evaluating the side effect of not sleeping > outside of the OOM path, despite you said Exactly. There is no point in preserving the status quo if you cannot reasonably argue of the effect. And I do not see the actual problem to be honest. If there are any, we can always fix up (note that OOM are rare events we do not optimize for) with the proper justification. > > If we _really_ need to touch should_reclaim_retry then > > it should be done in a separate patch with some numbers/tracing > > data backing that story. > > and I consider that "whether moving the short sleep to > should_reclaim_retry() has negative impact" and "whether > eliminating the short sleep has negative impact" should be > evaluated in a separate patch. > > but I will tolerate below patch if you can accept below patch "as-is" > (because it explicitly notes what actually happens and there might be > unexpected side effect of not sleeping outside of the OOM path). Well, I do not mind. If you really insist then I just do not care enough to argue. But please note that this very patch breaks your 1, 2, 3 :p So if you are really serious you should probably apply and test the following on top of Linus tree: diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 8ba6cb88cf58..ed9d473c571e 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -1077,15 +1077,9 @@ bool out_of_memory(struct oom_control *oc) dump_header(oc, NULL); panic("Out of memory and no killable processes...\n"); } - if (oc->chosen && oc->chosen != (void *)-1UL) { + if (oc->chosen && oc->chosen != (void *)-1UL) oom_kill_process(oc, !is_memcg_oom(oc) ? "Out of memory" : "Memory cgroup out of memory"); - /* - * Give the killed process a good chance to exit before trying - * to allocate memory again. - */ - schedule_timeout_killable(1); - } return !!oc->chosen; }