Received: by 2002:a05:6602:18e:0:0:0:0 with SMTP id m14csp2841018ioo; Tue, 24 May 2022 07:10:18 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwmGauU/dgBr/PGh2Zk30cWsogK/J6Xz/vvCBSsj9sY/KMBOI2eBV3EH691W6WpGILwpHmd X-Received: by 2002:a63:d505:0:b0:3c2:5a75:47f6 with SMTP id c5-20020a63d505000000b003c25a7547f6mr24275954pgg.170.1653401418553; Tue, 24 May 2022 07:10:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1653401418; cv=none; d=google.com; s=arc-20160816; b=KEVxcFaH2iU5G7W0AjzP8ZxNhQTuvzKg/mR+lab1meHrrQ5EYXWjm6dmUXyMpPJ5v6 2ADYncJGpfkZexC5BOLAJKxGEhMYDuL9Bbfa+lPMc8lB+LlI6wmZgbmPH5DTu9VgBNBZ vWUo4HG2NJyv5YF1Jz9R9yKNd8pWJ3D/2TrK/GITwcwQelE2CBkCKpAWP6+VofIlqiZ/ wxA2GdQkps/TFKdRN/3o0RKI27xDaFTLGA/XGd1RuyBqoKC8QdRgwCk5Cu5r1fvdcrin US03xal0np+VOmGsFKjXLIGTLIEAP9vErzsAgz/8ykxqSfsCpFxuh2OmA3APHOB4e+UI jDlg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=5hjdGDxm/5eb7UZfehykEkfZZM041fRa5GaKQ9lwQHI=; b=plXqOolgZZ9OvFAdEiX/tXm74fgY5zDUIXAq0adH99ToosRGqfd3yHVtrY/DjCVQUU QJ1jEC0w/1LohwKooGA5i0DvF4xAIUy0UpX3XLODeYg4PAG3gv1wJhJC7Sh0haGxiJsP 7CWHvu9cZMqQYw/WNOAgD9Kgci06d8B1g+/F5LTEmoDY9PzfrZNOj83hOxI1bWgj/RD7 Qtjwf16kBg7kxoxwB2RU59GCPpksKeTawPrt1EFB56ydzZl6mEOqtGGMUgqnFZb+L0Y+ icrOxNw3OjfGCVkIqMxx4jABYf59R6QncpS9ZVSTPdAyO41ILxs+f6poun5phkIUMRiW z+wA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=iKpecxYM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id j70-20020a638049000000b003fa0263081csi12749992pgd.726.2022.05.24.07.10.03; Tue, 24 May 2022 07:10:18 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=iKpecxYM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234263AbiEXKTQ (ORCPT + 99 others); Tue, 24 May 2022 06:19:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235957AbiEXKTL (ORCPT ); Tue, 24 May 2022 06:19:11 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 1BBEE8A336 for ; Tue, 24 May 2022 03:19:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1653387544; 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=5hjdGDxm/5eb7UZfehykEkfZZM041fRa5GaKQ9lwQHI=; b=iKpecxYMD7VCxtrbU+sPxVvd0BSKFPCnIGYphDjbIyqtrXmm7uONgJzKCgiq2l2C1zb1ED /eqwxKuc7BCt611qLNzwgMt1rlWEUTeCzgQN74FbOYZv9C6PUgx7VCkkvioUfM7nXLcpQv 9su6hBX4jKR9SNvklGaEN9jbKuR/20Q= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-13-cdRBusv8OiWHr2Ge-bjGGw-1; Tue, 24 May 2022 06:19:03 -0400 X-MC-Unique: cdRBusv8OiWHr2Ge-bjGGw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7889A101A52C; Tue, 24 May 2022 10:19:00 +0000 (UTC) Received: from localhost (ovpn-13-156.pek2.redhat.com [10.72.13.156]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2727240E7F0C; Tue, 24 May 2022 10:18:59 +0000 (UTC) Date: Tue, 24 May 2022 18:18:55 +0800 From: Baoquan He To: Petr Mladek Cc: "Guilherme G. Piccoli" , "michael Kelley (LINUX)" , Dave Young , d.hatayama@jp.fujitsu.com, akpm@linux-foundation.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, linuxppc-dev@lists.ozlabs.org, linux-alpha@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org, linux-hyperv@vger.kernel.org, linux-leds@vger.kernel.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, linux-pm@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-tegra@vger.kernel.org, linux-um@lists.infradead.org, linux-xtensa@linux-xtensa.org, netdev@vger.kernel.org, openipmi-developer@lists.sourceforge.net, rcu@vger.kernel.org, sparclinux@vger.kernel.org, xen-devel@lists.xenproject.org, x86@kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net, halves@canonical.com, fabiomirmar@gmail.com, alejandro.j.jimenez@oracle.com, andriy.shevchenko@linux.intel.com, arnd@arndb.de, bp@alien8.de, corbet@lwn.net, dave.hansen@linux.intel.com, feng.tang@intel.com, gregkh@linuxfoundation.org, hidehiro.kawai.ez@hitachi.com, jgross@suse.com, john.ogness@linutronix.de, keescook@chromium.org, luto@kernel.org, mhiramat@kernel.org, mingo@redhat.com, paulmck@kernel.org, peterz@infradead.org, rostedt@goodmis.org, senozhatsky@chromium.org, stern@rowland.harvard.edu, tglx@linutronix.de, vgoyal@redhat.com, vkuznets@redhat.com, will@kernel.org Subject: Re: [PATCH 24/30] panic: Refactor the panic path Message-ID: References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-25-gpiccoli@igalia.com> <20220519234502.GA194232@MiWiFi-R3L-srv> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.11.54.1 X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/24/22 at 10:01am, Petr Mladek wrote: > On Fri 2022-05-20 08:23:33, Guilherme G. Piccoli wrote: > > On 19/05/2022 20:45, Baoquan He wrote: > > > [...] > > >> I really appreciate the summary skill you have, to convert complex > > >> problems in very clear and concise ideas. Thanks for that, very useful! > > >> I agree with what was summarized above. > > > > > > I want to say the similar words to Petr's reviewing comment when I went > > > through the patches and traced each reviewing sub-thread to try to > > > catch up. Petr has reivewed this series so carefully and given many > > > comments I want to ack immediately. > > > > > > I agree with most of the suggestions from Petr to this patch, except of > > > one tiny concern, please see below inline comment. > > > > Hi Baoquan, thanks! I'm glad you're also reviewing that =) > > > > > > > [...] > > > > > > I like the proposed skeleton of panic() and code style suggested by > > > Petr very much. About panic_prefer_crash_dump which might need be added, > > > I hope it has a default value true. This makes crash_dump execute at > > > first by default just as before, unless people specify > > > panic_prefer_crash_dump=0|n|off to disable it. Otherwise we need add > > > panic_prefer_crash_dump=1 in kernel and in our distros to enable kdump, > > > this is inconsistent with the old behaviour. > > > > I'd like to understand better why the crash_kexec() must always be the > > first thing in your use case. If we keep that behavior, we'll see all > > sorts of workarounds - see the last patches of this series, Hyper-V and > > PowerPC folks hardcoded "crash_kexec_post_notifiers" in order to force > > execution of their relevant notifiers (like the vmbus disconnect, > > specially in arm64 that has no custom machine_crash_shutdown, or the > > fadump case in ppc). This led to more risk in kdump. > > > > The thing is: with the notifiers' split, we tried to keep only the most > > relevant/necessary stuff in this first list, things that ultimately > > should improve kdump reliability or if not, at least not break it. My > > feeling is that, with this series, we should change the idea/concept > > that kdump must run first nevertheless, not matter what. We're here > > trying to accommodate the antagonistic goals of hypervisors that need > > some clean-up (even for kdump to work) VS. kdump users, that wish a > > "pristine" system reboot ASAP after the crash. > > Good question. I wonder if Baoquan knows about problems caused by the > particular notifiers that will end up in the hypervisor list. Note > that there will be some shuffles and the list will be slightly > different in V2. Yes, I knew some of them. Please check my response to Guilherme. We have bug to track the issue on Hyper-V in which failure happened during panic notifiers running, haven't come to kdump. Seems both of us sent mail replying to Guilherme at the same time. > > Anyway, I see four possible solutions: > > 1. The most conservative approach is to keep the current behavior > and call kdump first by default. > > 2. A medium conservative approach to change the default default > behavior and call hypervisor and eventually the info notifiers > before kdump. There still would be the possibility to call kdump > first by the command line parameter. > > 3. Remove the possibility to call kdump first completely. It would > assume that all the notifiers in the info list are super safe > or that they make kdump actually more safe. > > 4. Create one more notifier list for operations that always should > be called before crash_dump. I would vote for 1 or 4 without any hesitation, and prefer 4. I ever suggest the variant of solution 4 in v1 reviewing. That's taking those notifiers out of list and enforcing to execute them before kdump. E.g the one on HyperV to terminate VMbus connection. Maybe solution 4 is better to provide a determinate way for people to add necessary code at the earliest part. > > Regarding the extra notifier list (4th solution). It is not clear to > me whether it would be always called even before hypervisor list or > when kdump is not enabled. We must not over-engineer it. One thing I would like to notice is, no matter how perfect we split the lists this time, we can't gurantee people will add notifiers reasonablly in the future. And people from different sub-component may not do sufficient investigation and add them to fulfil their local purpose. The current panic notifers list is the best example. Hyper-V actually wants to run some necessary code before kdump, but not all of them, they just add it, ignoring the original purpose of crash_kexec_post_notifiers. I guess they do like this just because it's easy to do, no need to bother changing code in generic place. Solution 4 can make this no doubt, that's why I like it better. > > 2nd proposal looks like a good compromise. But maybe we could do > this change few releases later. The notifiers split is a big > change on its own. As I replied to Guilherme, solution 2 will cause regression if not calling kdump firstly. Solution 3 leaves people space to make mistake, they could add nontifier into wrong list. I would like to note again that the panic notifiers are optional to run, while kdump is expectd once loaded, from the original purpose. I guess people I know will still have this thought, e.g Hatayama, Masa, they are truly often use panic notifiers like this on their company's system.