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 X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CD34DC433DB for ; Fri, 26 Feb 2021 15:40:43 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6D46064EC4 for ; Fri, 26 Feb 2021 15:40:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6D46064EC4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=xmission.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Subject:MIME-Version:Message-ID:In-Reply-To:Date: References:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=fq6PRmSZQWUEkQeOAzOj2GIj0hIxb1YDb6pDaiDFpTw=; b=fSTDbTEIfsbPWoKSlvkeyDLep j+yMJxAzhHvsO9PMDpsjqVJvCWXixoDUMeeEcpOoNPBB1ET9WFwuNlstmWb/V91MNK/iKUeuvn4cT 71hjgIEKjvigPz4S48R8bpIM7WIIJI8MjMKoFVYxfyOs++s3Cd4ILFt1jLyWdXwu5DvMr7ofD7SJo dGHcNPrU20+K+KWMBCvKdtHwa4mirPJzSd6VpQmQtFMdorBfkLudo/qxMkIMn9dwDOOt/j0pzzmYz yUEFpKi9S52pSfZXgc7If47TNaLulTfgHIxIHY9JvPcknsaMSForKeC0PGlSNMnjlcGtTDr65PktF ovvj5sjiQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lFfDH-00060S-Bs; Fri, 26 Feb 2021 15:39:31 +0000 Received: from out01.mta.xmission.com ([166.70.13.231]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1lFfDD-0005yk-P4; Fri, 26 Feb 2021 15:39:28 +0000 Received: from in02.mta.xmission.com ([166.70.13.52]) by out01.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1lFfCV-000SD1-1a; Fri, 26 Feb 2021 08:38:43 -0700 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=fess.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1lFfCU-000gGq-17; Fri, 26 Feb 2021 08:38:42 -0700 From: ebiederm@xmission.com (Eric W. Biederman) To: chenzhou References: <20210130071025.65258-1-chenzhou10@huawei.com> <20210130071025.65258-2-chenzhou10@huawei.com> <20210224141939.GA28965@arm.com> <20210225072426.GH3553@MiWiFi-R3L-srv> <121fa1e6-f1a3-d47f-bb1d-baaacf96fddc@huawei.com> Date: Fri, 26 Feb 2021 09:38:37 -0600 In-Reply-To: <121fa1e6-f1a3-d47f-bb1d-baaacf96fddc@huawei.com> (chenzhou's message of "Fri, 26 Feb 2021 14:45:25 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1lFfCU-000gGq-17; ; ; mid=; ; ; hst=in02.mta.xmission.com; ; ; ip=68.227.160.95; ; ; frm=ebiederm@xmission.com; ; ; spf=neutral X-XM-AID: U2FsdGVkX19FonAfyXHc11Z6qU3W8XfsdQm1v4+5X8w= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v14 01/11] x86: kdump: replace the hard-coded alignment with macro CRASH_ALIGN X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210226_103927_828465_3DF6F52E X-CRM114-Status: GOOD ( 18.57 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: wangkefeng.wang@huawei.com, linux-doc@vger.kernel.org, Catalin Marinas , huawei.libin@huawei.com, guohanjun@huawei.com, will@kernel.org, Baoquan He , corbet@lwn.net, mingo@redhat.com, dyoung@redhat.com, John.P.donnelly@oracle.com, arnd@arndb.de, xiexiuqi@huawei.com, horms@verge.net.au, tglx@linutronix.de, linux-arm-kernel@lists.infradead.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, robh+dt@kernel.org, james.morse@arm.com, rppt@kernel.org, prabhakar.pkin@gmail.com, nsaenzjulienne@suse.de 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 chenzhou writes: > On 2021/2/25 15:25, Baoquan He wrote: >> On 02/24/21 at 02:19pm, Catalin Marinas wrote: >>> On Sat, Jan 30, 2021 at 03:10:15PM +0800, Chen Zhou wrote: >>>> Move CRASH_ALIGN to header asm/kexec.h for later use. Besides, the >>>> alignment of crash kernel regions in x86 is 16M(CRASH_ALIGN), but >>>> function reserve_crashkernel() also used 1M alignment. So just >>>> replace hard-coded alignment 1M with macro CRASH_ALIGN. >>> [...] >>>> @@ -510,7 +507,7 @@ static void __init reserve_crashkernel(void) >>>> } else { >>>> unsigned long long start; >>>> >>>> - start = memblock_phys_alloc_range(crash_size, SZ_1M, crash_base, >>>> + start = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, >>>> crash_base + crash_size); >>>> if (start != crash_base) { >>>> pr_info("crashkernel reservation failed - memory is in use.\n"); >>> There is a small functional change here for x86. Prior to this patch, >>> crash_base passed by the user on the command line is allowed to be 1MB >>> aligned. With this patch, such reservation will fail. >>> >>> Is the current behaviour a bug in the current x86 code or it does allow >>> 1MB-aligned reservations? >> Hmm, you are right. Here we should keep 1MB alignment as is because >> users specify the address and size, their intention should be respected. >> The 1MB alignment for fixed memory region reservation was introduced in >> below commit, but it doesn't tell what is Eric's request at that time, I >> guess it meant respecting users' specifying. > I think we could make the alignment unified. Why is the alignment system reserved and > user specified different? Besides, there is no document about the 1MB alignment. > How about adding the alignment size(16MB) in doc if user specified > start address as arm64 does. Looking at what the code is doing. Attempting to reserve a crash region at the location the user specified. Adding unnecessary alignment constraints is totally broken. I am not even certain enforcing a 1MB alignment makes sense. I suspect it was added so that we don't accidentally reserve low memory on x86. Frankly I am not even certain that makes sense. Now in practice there might be an argument for 2MB alignment that goes with huge page sizes on x86. But until someone finds that there are actual problems with 1MB alignment I would not touch it. The proper response to something that isn't documented and confusing is not to arbitrarily change it and risk breaking users. Especially in this case where it is clear that adding additional alignment is total nonsense. The proper response to something that isn't clear and documented is to dig in and document it, or to leave it alone and let it be the next persons problem. In this case there is no reason for changing this bit of code. All CRASH_ALIGN is about is a default alignment when none is specified. It is not a functional requirement but just something so that things come out nicely. Eric _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel