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 DE592C77B6C for ; Fri, 7 Apr 2023 12:59:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233543AbjDGM7D (ORCPT ); Fri, 7 Apr 2023 08:59:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37164 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231247AbjDGM7B (ORCPT ); Fri, 7 Apr 2023 08:59:01 -0400 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 795B476BB; Fri, 7 Apr 2023 05:58:58 -0700 (PDT) Received: from dggpemm500006.china.huawei.com (unknown [172.30.72.56]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4PtJHY3gVrzSmPS; Fri, 7 Apr 2023 20:55:05 +0800 (CST) Received: from [10.174.178.55] (10.174.178.55) by dggpemm500006.china.huawei.com (7.185.36.236) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Fri, 7 Apr 2023 20:58:54 +0800 Subject: Re: [PATCH -next v3 1/2] riscv: kdump: Implement crashkernel=X,[high,low] To: Simon Horman , Chen Jiahao CC: , , , , , , , , , , , , , References: <20230406220206.3067006-1-chenjiahao16@huawei.com> <20230406220206.3067006-2-chenjiahao16@huawei.com> From: "Leizhen (ThunderTown)" Message-ID: Date: Fri, 7 Apr 2023 20:58:53 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.178.55] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggpemm500006.china.huawei.com (7.185.36.236) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2023/4/7 20:03, Simon Horman wrote: > On Fri, Apr 07, 2023 at 06:02:05AM +0800, Chen Jiahao wrote: >> On riscv, the current crash kernel allocation logic is trying to >> allocate within 32bit addressible memory region by default, if >> failed, try to allocate without 4G restriction. >> >> In need of saving DMA zone memory while allocating a relatively large >> crash kernel region, allocating the reserved memory top down in >> high memory, without overlapping the DMA zone, is a mature solution. >> Here introduce the parameter option crashkernel=X,[high,low]. >> >> One can reserve the crash kernel from high memory above DMA zone range >> by explicitly passing "crashkernel=X,high"; or reserve a memory range >> below 4G with "crashkernel=X,low". >> >> Signed-off-by: Chen Jiahao > > ... > >> @@ -1180,14 +1206,37 @@ static void __init reserve_crashkernel(void) >> return; >> } >> >> - ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(), >> + ret = parse_crashkernel(cmdline, memblock_phys_mem_size(), >> &crash_size, &crash_base); >> - if (ret || !crash_size) >> + if (ret == -ENOENT) { >> + /* >> + * crashkernel=X,[high,low] can be specified or not, but >> + * invalid value is not allowed. > > nit: Perhaps something like this would be easier to correlate with the > code that follows: > > /* Fallback to crashkernel=X,[high,low] */ The description "crashkernel=X,[high,low] can be specified or not" is not correct, because crashkernel=X,high must be specified when walking into this branch. So use Simon's comments or copy arm64's comments(it's written for parse_crashkernel_low()). > > >> + */ >> + ret = parse_crashkernel_high(cmdline, 0, &crash_size, &crash_base); >> + if (ret || !crash_size) >> + return; >> + >> + /* >> + * crashkernel=Y,low is valid only when crashkernel=X,high >> + * is passed and high memory is reserved successful. > > nit: s/successful/successfully/ Seems like the whole "and high memory is reserved successful" needs to be deleted. Only the dependency between the two boot options should be described here, regardless of whether their memory is successfully allocated. > >> + */ >> + ret = parse_crashkernel_low(cmdline, 0, &crash_low_size, &crash_base); >> + if (ret == -ENOENT) >> + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE; >> + else if (ret) >> + return; >> + >> + search_start = search_low_max; >> + } else if (ret || !crash_size) { >> + /* Invalid argument value specified */ >> return; >> + } > > ... > . > -- Regards, Zhen Lei