Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932138AbbFIWpq (ORCPT ); Tue, 9 Jun 2015 18:45:46 -0400 Received: from mail-ie0-f182.google.com ([209.85.223.182]:35669 "EHLO mail-ie0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752587AbbFIWph (ORCPT ); Tue, 9 Jun 2015 18:45:37 -0400 Date: Tue, 9 Jun 2015 15:45:35 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Michal Hocko cc: Austin S Hemmelgarn , Andrew Morton , linux-mm@kvack.org, LKML Subject: Re: [PATCH] oom: split out forced OOM killer In-Reply-To: <20150609093659.GA29057@dhcp22.suse.cz> Message-ID: References: <1433235187-32673-1-git-send-email-mhocko@suse.cz> <557187F9.8020301@gmail.com> <5575E5E6.20908@gmail.com> <20150608210621.GA18360@dhcp22.suse.cz> <20150609093659.GA29057@dhcp22.suse.cz> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1811 Lines: 40 On Tue, 9 Jun 2015, Michal Hocko wrote: > > Yes, and that's why I believe we should pursue that direction without the > > associated "cleanup" that adds 35 lines of code to supress a panic. In > > other words, there's no reason to combine a patch that suppresses the > > panic even with panic_on_oom, which I support, and a "cleanup" that I > > believe just obfuscates the code. > > > > It's a one-liner change: just test for force_kill and suppress the panic; > > we don't need 35 new lines that create even more unique entry paths. > > I completely detest yet another check in out_of_memory. And there is > even no reason to do that. Forced kill and genuine oom have different > objectives and combining those two just makes the code harder to read > (one has to go to check the syrq callback to realize that the forced > path is triggered from the workqueue context and that current->mm != > NULL check will prevent some heuristics. This is just too ugly to > live). So why the heck are you pushing for keeping everything in a > single path? > Perhaps if you renamed "force_kill" to "sysrq" it would make more sense to you? I don't think the oom killer needs multiple entry points that duplicates code and adds more than twice the lines it removes. It would make sense if that was an optimization in a hot path, or a warm path, or even a luke-warm path, but not an icy cold path like the oom killer. check_panic_on_oom() can simply do if (sysrq) return; It's not hard and it's very clear. We don't need 35 more lines of code to do this. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/