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 D80F5C77B6F for ; Sat, 8 Apr 2023 02:01:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229834AbjDHCBA (ORCPT ); Fri, 7 Apr 2023 22:01:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52574 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229544AbjDHCA7 (ORCPT ); Fri, 7 Apr 2023 22:00:59 -0400 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3B209CA32; Fri, 7 Apr 2023 19:00:57 -0700 (PDT) Received: from dggpemm500006.china.huawei.com (unknown [172.30.72.55]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4PtdfG0fWTznYd8; Sat, 8 Apr 2023 09:57:26 +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; Sat, 8 Apr 2023 10:00:54 +0800 Subject: Re: [PATCH -next v3 1/2] riscv: kdump: Implement crashkernel=X,[high,low] From: "Leizhen (ThunderTown)" To: Simon Horman , Chen Jiahao CC: , , , , , , , , , , , , , References: <20230406220206.3067006-1-chenjiahao16@huawei.com> <20230406220206.3067006-2-chenjiahao16@huawei.com> Message-ID: Date: Sat, 8 Apr 2023 10:00: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: dggems701-chm.china.huawei.com (10.3.19.178) 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:58, Leizhen (ThunderTown) wrote: > > > 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()). I rethink it a little bit, if it's relative to crashkernel=X[@offset], that's also true. Reviewed-by: Zhen Lei > >> >> >>> + */ >>> + 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