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=-12.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 8891DC433ED for ; Sat, 8 May 2021 03:29:51 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CF5786144F for ; Sat, 8 May 2021 03:29:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CF5786144F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C1B478D0024; Fri, 7 May 2021 23:29:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BF2B48D001A; Fri, 7 May 2021 23:29:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A6B788D0024; Fri, 7 May 2021 23:29:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0222.hostedemail.com [216.40.44.222]) by kanga.kvack.org (Postfix) with ESMTP id 8BBDC8D001A for ; Fri, 7 May 2021 23:29:49 -0400 (EDT) Received: from smtpin22.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 41F166130 for ; Sat, 8 May 2021 03:29:49 +0000 (UTC) X-FDA: 78116634498.22.7D53C8A Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf12.hostedemail.com (Postfix) with ESMTP id CCFC2F4 for ; Sat, 8 May 2021 03:29:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620444588; 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: in-reply-to:in-reply-to:references:references; bh=OJNHric203NrDGObIcSAdis+ynJzNtimirkRgG0nbHs=; b=VZ/wjSx8qjOWm7zSsxe9374AJx75QO/z5yHLwhewoAGheIt3w6H4tscKumjBBrOJ4fi6fF /oDDiXdNkF/bTh07hEPuegmqwIkAKkPfcCoHOotilGbfNMlhIymz2o5qXs4XT1v5QPjBrQ a/CU2c8JHRy0/hFP3t/6WRCD1gR0ZVY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-196-mndJ_4eSPZqLxIrrgXqxRw-1; Fri, 07 May 2021 23:29:46 -0400 X-MC-Unique: mndJ_4eSPZqLxIrrgXqxRw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 89B1A1854E26; Sat, 8 May 2021 03:29:43 +0000 (UTC) Received: from localhost (ovpn-13-42.pek2.redhat.com [10.72.13.42]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0BACD687F0; Sat, 8 May 2021 03:29:39 +0000 (UTC) Date: Sat, 8 May 2021 11:29:36 +0800 From: Baoquan He To: Linus Torvalds Cc: Andrew Morton , Andrey Konovalov , Christian Brauner , Colin King , Jonathan Corbet , dyoung@redhat.com, Frederic Weisbecker , gpiccoli@canonical.com, john.p.donnelly@oracle.com, Josh Poimboeuf , Kees Cook , Linux-MM , Masahiro Yamada , Mauro Carvalho Chehab , Mike Kravetz , Ingo Molnar , mm-commits@vger.kernel.org, "Paul E. McKenney" , Peter Zijlstra , Randy Dunlap , Steven Rostedt , Mike Rapoport , saeed.mirzamohammadi@oracle.com, Sami Tolvanen , Stephen Boyd , Thomas Gleixner , Vivek Goyal , yifeifz2@illinois.edu, Hari Bathini , piliu@redhat.com, kasong@redhat.com Subject: Re: [patch 48/91] kernel/crash_core: add crashkernel=auto for vmcore creation Message-ID: <20210508032936.GC23668@MiWiFi-R3L-srv> References: <20210506180126.03e1baee7ca52bedb6cc6003@linux-foundation.org> <20210507010432.IN24PudKT%akpm@linux-foundation.org> <20210508031309.GD2834@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210508031309.GD2834@localhost.localdomain> User-Agent: Mutt/1.10.1 (2018-07-13) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="VZ/wjSx8"; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf12.hostedemail.com: domain of bhe@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=bhe@redhat.com X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: CCFC2F4 X-Stat-Signature: zr138tpquzft53giab6nry4zga4na8gh Received-SPF: none (redhat.com>: No applicable sender policy available) receiver=imf12; identity=mailfrom; envelope-from=""; helo=us-smtp-delivery-124.mimecast.com; client-ip=170.10.133.124 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1620444573-447911 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Add Kairui to CC since he is taking care of the crashkernel=auto code in our Distros. On 05/08/21 at 11:13am, Baoquan He wrote: > Hi Linus, > > On 05/07/21 at 12:25am, Linus Torvalds wrote: > > On Thu, May 6, 2021 at 6:04 PM Andrew Morton wrote: > > > > > > From: Saeed Mirzamohammadi > > > Subject: kernel/crash_core: add crashkernel=auto for vmcore creation > > > > > > This adds crashkernel=auto feature to configure reserved memory for vmcore > > > creation. CONFIG_CRASH_AUTO_STR is defined to be set for different kernel > > > distributions and different archs based on their needs. > > > > > > Ugh. I didn't realize how nasty this was until after I'd applied this patch. > > > > I'm going to drop this patch, because the Kconfig thing for it is an > > unmitigated mess. I was confused by the question, and then the help > > message was actively misleading. > > > > This is wrong for so many reasons: > > > > - this is a classic case of "you shouldn't ask a user this". > > > > The question makes no sense to any normal person, it certainly > > didn't to me. Don't ask questions that don't have sane answers. > > > > - the config help text is actively misleading, and claims that the > > option is about how much memory is reserved for a crash kernel > > > > Not so. It's the default string for when somebody uses "crashkernel=auto" > > Sorry for the confusion, we should have been more careful to reivew and > add the commit log and kernel config description. > > > > - this shouldn't be a config option at all, it's clearly a distro > > setting, and should be on the kernel command line with the other > > distro settings. > > Don't know kernel config is disliked sometime, will remember it in the > future and more cautiously to add. > > Crashkernel=auto exists in our distros for many years, and as David > mentioned in other thread, we have been trying to adding rashkernel=auto > support into upstream. We pursue crashkernel=auto being added to upstream > because: > > 1) Empirical value is given to user by default; > > It was required by customer originally, now has been an important part > of kdump feature and supported in several main ARCHes. With crashkernel=auto, > people w/o much knowledge of kdump details can use kdump to debug. Distros > can provide the suggested values with crashkernel=auto which are got by > investigation, analysis and tested widely on test environment. > > 2) Cover corner case/special case; > > In some cases, kernel may need extra memory to handle, kdump kernel is > not exceptional. E.g when sme/sev enabled, SWIOTLB will be enabled > necessarily, even in kdump kernel. (Below sme/sev related commits for > reference). Then extra 64M need be reserved for crashkernel. User > doesn't need to know this, we already have done it for them. > > commit c7753208a94c ("x86, swiotlb: Add memory encryption support") > commit aba2d9a6385a ("iommu/amd: Do not disable SWIOTLB if SME is active") > commit d7b417fa08d1 ("x86/mm: Add DMA support for SEV memory encryption") > > We are eager to push crashkernel=auto to upstream becasue of our > UPSTREAM FIRST rule. Since it has been in RHEL for many years, each time > a new RHEL main release anchor a upstream kernel release and is prepared, > these crashkernel=auto RHEL-only patches need be reviewed inside Redhat, > then we will be questioned and challenged why they are not in upstream. > > As for how to implement crashkernel=auto, we have tried several ways. > > 1) Add into kernel command line > > The suggested value need be stored in user space if added into kernel > command line, then added into kernel. This makes the suggested value > separated from kernel itself. It's not what we expect to see. Because > the suggested crashkernel value is strongly related to distros release. > We could adjust the value between sub-releases of kernel because of > of kernel change. Adding them into kernel command line make us lose the > track of them in kernel. > > 2) Add a weak generic function and several arch dependent functions > 3) Hardcode values in __parse_crashkernel() > > Method 2) is taken in our RHEL7, 3) is used in RHEL8, RHEL-only patches > add them. If we try to push them into upstram, any later value > adjustment need a upstream patch posting. Otherwise, RHEL-only patch > need be introduced again, Redhat internal reviewer will challenge us > again. (Put the value hard coding pieces at bottom for reference). > > 4) Add kernel config to add default value > > It's done in this patch. With the kernel config CRASH_AUTO_STR, Distros can > add default value, and adjust it anytime in the future w/o bothering > upstream. If crashkernel=auto is specified, only below 3 LOC added, to > go to parse the CONFIG_CRASH_AUTO_STR directly. > > @@ -250,6 +251,12 @@ static int __init __parse_crashkernel(ch > if (suffix) > return parse_crashkernel_suffix(ck_cmdline, crash_size, > suffix); > +#ifdef CONFIG_CRASH_AUTO_STR > + if (strncmp(ck_cmdline, "auto", 4) == 0) { > + ck_cmdline = CONFIG_CRASH_AUTO_STR; > + pr_info("Using crashkernel=auto, the size chosen is a best effort estimation.\n"); > + } > +#endif > > > Before this, we don't know Saeed Mirzamohammadi, the patch author. He > could experience the same torture. We were wild with joy when noticing > his patch. We were planning to launch new round of post to add > crashkernel=auto, kernel config is our final option too. We could be too > happy to forget polishing the commit log. > > Not sure if I make myself clear. Basically, we expect crashkernel=auto > to be added in upstream kernel. About how to implement it in kernel, we would > like to hear upstream people's suggestion. > > Thanks > Baoquan > > > Hard code crashkernel=auto values in __parse_crashkernel() > =========================================================== > static int __init __parse_crashkernel(char *cmdline, > unsigned long long system_ram, > unsigned long long *crash_size, > unsigned long long *crash_base, > const char *name, > const char *suffix) > { > ...... > if (strncmp(ck_cmdline, "auto", 4) == 0) { > #if defined(CONFIG_X86_64) || defined(CONFIG_S390) > ck_cmdline = "1G-4G:160M,4G-64G:192M,64G-1T:256M,1T-:512M"; > #elif defined(CONFIG_ARM64) > ck_cmdline = "2G-:448M"; > #elif defined(CONFIG_PPC64) > char *fadump_cmdline; > > fadump_cmdline = get_last_crashkernel(cmdline, "fadump=", NULL); > fadump_cmdline = fadump_cmdline ? > fadump_cmdline + strlen("fadump=") : NULL; > if (!fadump_cmdline || (strncmp(fadump_cmdline, "off", 3) == 0)) > ck_cmdline = "2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G"; > else > ck_cmdline = "4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"; > #endif > pr_info("Using crashkernel=auto, the size chosen is a best effort estimation.\n"); > } > > ...... > } > ================================================================== > > > >