From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BBBB2C433FE for ; Tue, 28 Dec 2021 16:13:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235675AbhL1QNM (ORCPT ); Tue, 28 Dec 2021 11:13:12 -0500 Received: from mail.skyhub.de ([5.9.137.197]:48190 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235511AbhL1QNK (ORCPT ); Tue, 28 Dec 2021 11:13:10 -0500 Received: from zn.tnic (dslb-088-067-202-008.088.067.pools.vodafone-ip.de [88.67.202.8]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.skyhub.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id 5B0001EC03C9; Tue, 28 Dec 2021 17:13:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1640707984; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:in-reply-to:in-reply-to: references:references; bh=WXiQ34aw3BVND/Em7tvSPxlLMgOR1j97eSJLIoe4hEo=; b=Wxfh2AfVkOPehtchcQkGOoQ0oGIuT8TcGQj16+fz/HtvWax3OrT7PDYc6oNz8WRzjBQVGz Y5x48biGa9AeRsmxuPHUMmhPc/e5BbRXINa6EOML0ucxMIFvD84s6QGJryKawaDfVG69Fh DDSVwawRqdPXycOFmaGv7WJE5dupzC8= Date: Tue, 28 Dec 2021 17:13:06 +0100 From: Borislav Petkov To: Zhen Lei Cc: Thomas Gleixner , Ingo Molnar , x86@kernel.org, "H . Peter Anvin" , linux-kernel@vger.kernel.org, Dave Young , Baoquan He , Vivek Goyal , Eric Biederman , kexec@lists.infradead.org, Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, Rob Herring , Frank Rowand , devicetree@vger.kernel.org, Jonathan Corbet , linux-doc@vger.kernel.org, Randy Dunlap , Feng Zhou , Kefeng Wang , Chen Zhou , John Donnelly Subject: Re: [PATCH v19 02/13] x86/setup: Use parse_crashkernel_high_low() to simplify code Message-ID: References: <20211228132612.1860-1-thunder.leizhen@huawei.com> <20211228132612.1860-3-thunder.leizhen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20211228132612.1860-3-thunder.leizhen@huawei.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 28, 2021 at 09:26:01PM +0800, Zhen Lei wrote: > Use parse_crashkernel_high_low() to bring the parsing of > "crashkernel=X,high" and the parsing of "crashkernel=Y,low" together, they > are strongly dependent, make code logic clear and more readable. > > Suggested-by: Borislav Petkov Yeah, doesn't look like something I suggested... > @@ -474,10 +472,9 @@ static void __init reserve_crashkernel(void) > /* crashkernel=XM */ > ret = parse_crashkernel(boot_command_line, total_mem, &crash_size, &crash_base); > if (ret != 0 || crash_size <= 0) { > - /* crashkernel=X,high */ > - ret = parse_crashkernel_high(boot_command_line, total_mem, > - &crash_size, &crash_base); > - if (ret != 0 || crash_size <= 0) > + /* crashkernel=X,high and possible crashkernel=Y,low */ > + ret = parse_crashkernel_high_low(boot_command_line, &crash_size, &low_size); So this calls parse_crashkernel() and when that one fails, it calls this new weird parse high/low helper you added. But then all three end up in the same __parse_crashkernel() worker function which seems to do the actual parsing. What I suggested and what would be real clean is if the arches would simply call a *single* parse_crashkernel() function and when that one returns, *all* crashkernel= options would have been parsed properly, low, high, middle crashkernel, whatever... and the caller would know what crash kernel needs to be allocated. Then each arch can do its memory allocations and checks based on that parsed data and decide to allocate or bail. So it is getting there but it needs more surgery... Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 04734C433EF for ; Tue, 28 Dec 2021 16:14:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=1iygUqM9DEzrYITS08D5tXG2gv0yn/jeg1f3URtADAs=; b=hkx0QPIejZPajW 7AjBml/M2l75DenG0ilVBtVv3vbDU5+m9e4ehxKvyGirdlwjhGVUkJ9PvW2MGpdnwQJT6jUr/+GiY ECSLah7tdnkTJxa2NavKcbcSiOeoi9tQbKLVd7qRbevU8IbaeOggbMC6vAWMdMhFd7d0Gzl+GZlpJ yr19UtQmsJpr+r00E0bqxriw1AqO/IzeCMa431RnZYRe2tkYLYwVctBIMiwQQ8X9MIEcSUg7NylCV PfrrqoPoRa0AIPKVPPpN9UwlFONsM9vkMnoI8R3o/QZFEm58lv/FWXlV7u86kTkC3fJjYE1flrHp8 7tR4sFopc/B64+S8egJg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n2F6J-001UmS-3o; Tue, 28 Dec 2021 16:13:23 +0000 Received: from mail.skyhub.de ([5.9.137.197]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n2F67-001UkB-6R; Tue, 28 Dec 2021 16:13:12 +0000 Received: from zn.tnic (dslb-088-067-202-008.088.067.pools.vodafone-ip.de [88.67.202.8]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.skyhub.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id 5B0001EC03C9; Tue, 28 Dec 2021 17:13:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1640707984; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:in-reply-to:in-reply-to: references:references; bh=WXiQ34aw3BVND/Em7tvSPxlLMgOR1j97eSJLIoe4hEo=; b=Wxfh2AfVkOPehtchcQkGOoQ0oGIuT8TcGQj16+fz/HtvWax3OrT7PDYc6oNz8WRzjBQVGz Y5x48biGa9AeRsmxuPHUMmhPc/e5BbRXINa6EOML0ucxMIFvD84s6QGJryKawaDfVG69Fh DDSVwawRqdPXycOFmaGv7WJE5dupzC8= Date: Tue, 28 Dec 2021 17:13:06 +0100 From: Borislav Petkov To: Zhen Lei Cc: Thomas Gleixner , Ingo Molnar , x86@kernel.org, "H . Peter Anvin" , linux-kernel@vger.kernel.org, Dave Young , Baoquan He , Vivek Goyal , Eric Biederman , kexec@lists.infradead.org, Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, Rob Herring , Frank Rowand , devicetree@vger.kernel.org, Jonathan Corbet , linux-doc@vger.kernel.org, Randy Dunlap , Feng Zhou , Kefeng Wang , Chen Zhou , John Donnelly Subject: Re: [PATCH v19 02/13] x86/setup: Use parse_crashkernel_high_low() to simplify code Message-ID: References: <20211228132612.1860-1-thunder.leizhen@huawei.com> <20211228132612.1860-3-thunder.leizhen@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211228132612.1860-3-thunder.leizhen@huawei.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211228_081311_402272_2CA23D2E X-CRM114-Status: GOOD ( 17.74 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Dec 28, 2021 at 09:26:01PM +0800, Zhen Lei wrote: > Use parse_crashkernel_high_low() to bring the parsing of > "crashkernel=X,high" and the parsing of "crashkernel=Y,low" together, they > are strongly dependent, make code logic clear and more readable. > > Suggested-by: Borislav Petkov Yeah, doesn't look like something I suggested... > @@ -474,10 +472,9 @@ static void __init reserve_crashkernel(void) > /* crashkernel=XM */ > ret = parse_crashkernel(boot_command_line, total_mem, &crash_size, &crash_base); > if (ret != 0 || crash_size <= 0) { > - /* crashkernel=X,high */ > - ret = parse_crashkernel_high(boot_command_line, total_mem, > - &crash_size, &crash_base); > - if (ret != 0 || crash_size <= 0) > + /* crashkernel=X,high and possible crashkernel=Y,low */ > + ret = parse_crashkernel_high_low(boot_command_line, &crash_size, &low_size); So this calls parse_crashkernel() and when that one fails, it calls this new weird parse high/low helper you added. But then all three end up in the same __parse_crashkernel() worker function which seems to do the actual parsing. What I suggested and what would be real clean is if the arches would simply call a *single* parse_crashkernel() function and when that one returns, *all* crashkernel= options would have been parsed properly, low, high, middle crashkernel, whatever... and the caller would know what crash kernel needs to be allocated. Then each arch can do its memory allocations and checks based on that parsed data and decide to allocate or bail. So it is getting there but it needs more surgery... Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Date: Tue, 28 Dec 2021 17:13:06 +0100 From: Borislav Petkov Subject: Re: [PATCH v19 02/13] x86/setup: Use parse_crashkernel_high_low() to simplify code Message-ID: References: <20211228132612.1860-1-thunder.leizhen@huawei.com> <20211228132612.1860-3-thunder.leizhen@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211228132612.1860-3-thunder.leizhen@huawei.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Zhen Lei Cc: Thomas Gleixner , Ingo Molnar , x86@kernel.org, "H . Peter Anvin" , linux-kernel@vger.kernel.org, Dave Young , Baoquan He , Vivek Goyal , Eric Biederman , kexec@lists.infradead.org, Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, Rob Herring , Frank Rowand , devicetree@vger.kernel.org, Jonathan Corbet , linux-doc@vger.kernel.org, Randy Dunlap , Feng Zhou , Kefeng Wang , Chen Zhou , John Donnelly On Tue, Dec 28, 2021 at 09:26:01PM +0800, Zhen Lei wrote: > Use parse_crashkernel_high_low() to bring the parsing of > "crashkernel=X,high" and the parsing of "crashkernel=Y,low" together, they > are strongly dependent, make code logic clear and more readable. > > Suggested-by: Borislav Petkov Yeah, doesn't look like something I suggested... > @@ -474,10 +472,9 @@ static void __init reserve_crashkernel(void) > /* crashkernel=XM */ > ret = parse_crashkernel(boot_command_line, total_mem, &crash_size, &crash_base); > if (ret != 0 || crash_size <= 0) { > - /* crashkernel=X,high */ > - ret = parse_crashkernel_high(boot_command_line, total_mem, > - &crash_size, &crash_base); > - if (ret != 0 || crash_size <= 0) > + /* crashkernel=X,high and possible crashkernel=Y,low */ > + ret = parse_crashkernel_high_low(boot_command_line, &crash_size, &low_size); So this calls parse_crashkernel() and when that one fails, it calls this new weird parse high/low helper you added. But then all three end up in the same __parse_crashkernel() worker function which seems to do the actual parsing. What I suggested and what would be real clean is if the arches would simply call a *single* parse_crashkernel() function and when that one returns, *all* crashkernel= options would have been parsed properly, low, high, middle crashkernel, whatever... and the caller would know what crash kernel needs to be allocated. Then each arch can do its memory allocations and checks based on that parsed data and decide to allocate or bail. So it is getting there but it needs more surgery... Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec