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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 01ECBC433EF for ; Wed, 10 Nov 2021 08:10:29 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 84F8F611C0 for ; Wed, 10 Nov 2021 08:10:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 84F8F611C0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id DBBAF6B006C; Wed, 10 Nov 2021 03:10:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D6AEA6B0071; Wed, 10 Nov 2021 03:10:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C327C6B0072; Wed, 10 Nov 2021 03:10:27 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0146.hostedemail.com [216.40.44.146]) by kanga.kvack.org (Postfix) with ESMTP id B6A7D6B006C for ; Wed, 10 Nov 2021 03:10:27 -0500 (EST) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 73A398249980 for ; Wed, 10 Nov 2021 08:10:27 +0000 (UTC) X-FDA: 78792298494.29.80AC9E3 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by imf19.hostedemail.com (Postfix) with ESMTP id 469C5B0000BE for ; Wed, 10 Nov 2021 08:10:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1636531826; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RYU0qEZT/fDbRdYvdmH1fDufqSqdn55jv1jkSYCFEeM=; b=DUYl2yEey5iSXmNrd3ALq25XcvCyR8++BYSX3Rd/VDYZISmADjOby2QESm1zFcGsRpKEaW uv0FmKKzt4lYwYERpLSW7NvxJe/ZF9wC1eP9X6UCREZsjHVpQphJ1ZJ8C3LuYwrKz6XBaY Yw+D79vD/UMwYJu4mBj+yoaxa08+dVo= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-399-ITRWGj_VNxmlMFg0OBRxbA-1; Wed, 10 Nov 2021 03:10:25 -0500 X-MC-Unique: ITRWGj_VNxmlMFg0OBRxbA-1 Received: by mail-wr1-f71.google.com with SMTP id h7-20020adfaa87000000b001885269a937so214958wrc.17 for ; Wed, 10 Nov 2021 00:10:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:organization:subject :in-reply-to:content-transfer-encoding; bh=RYU0qEZT/fDbRdYvdmH1fDufqSqdn55jv1jkSYCFEeM=; b=cNf5qTVSr0YMHKHj+ClPA9lPrIvaLf0aA/Svi57viOdh0ZqMUw+bwhU3mTxST2/tG2 wQPCBwo8CZSRMx53uuPkoP9tEzPzH4LecwO646qPoNyfwlepMNIYZoAgHJtZXVelZhKx y1sy0Jm9oEUXgaQ1qsFQZrZJK8PvwBmLid2QRuy7ynasZYvaKPpww/2HUyHDg5iOATq7 5rq1ELFqpv5LtGVu0RRo3vnxKCqRkIUY2MePQu7MrPHDgaVO68QMSGbgwKkQhp9tCibI fxTHgiRsHKbbd3GU5eUnvCcOLoAJDfnuE8H2RqhduwsaC9lq6gqdbnFKd34mxDvHvaEC Dy/Q== X-Gm-Message-State: AOAM531jlqUzBjePLbYDYK7BYm+UDIzvZo5/uXL4ep9JJo7tcxfvwjkZ KbrA7ECKXV77W21Tz/nElpeVq6DhEneDvFhXXeBpLgIt83X06Bd11FtsSheTS/qf24ZWPAuP+Of SioKIi95C+TM= X-Received: by 2002:a5d:4411:: with SMTP id z17mr16957755wrq.59.1636531824088; Wed, 10 Nov 2021 00:10:24 -0800 (PST) X-Google-Smtp-Source: ABdhPJzpGKiHsebg1C7WcrJxd78Q4PUsjBiNMRA5HfzoU8b0qvs/dNWNzwHAAHvscsds6ojRfHHgSw== X-Received: by 2002:a5d:4411:: with SMTP id z17mr16957705wrq.59.1636531823797; Wed, 10 Nov 2021 00:10:23 -0800 (PST) Received: from [192.168.3.132] (p5b0c604f.dip0.t-ipconnect.de. [91.12.96.79]) by smtp.gmail.com with ESMTPSA id i15sm4778576wmb.20.2021.11.10.00.10.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 10 Nov 2021 00:10:23 -0800 (PST) Message-ID: <0c68b366-38f4-94fd-da11-57e40a44cb48@redhat.com> Date: Wed, 10 Nov 2021 09:10:22 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 To: Baoquan He Cc: boris.ostrovsky@oracle.com, bp@alien8.de, Andrew Morton , dyoung@redhat.com, hpa@zytor.com, jasowang@redhat.com, jgross@suse.com, linux-mm@kvack.org, mhocko@suse.com, mingo@redhat.com, mm-commits@vger.kernel.org, mst@redhat.com, osalvador@suse.de, rafael.j.wysocki@intel.com, rppt@kernel.org, sstabellini@kernel.org, tglx@linutronix.de, torvalds@linux-foundation.org, vgoyal@redhat.com References: <20211108183057.809e428e841088b657a975ec@linux-foundation.org> <20211109023148.b1OlyuiXG%akpm@linux-foundation.org> <20211110072225.GA18768@MiWiFi-R3L-srv> From: David Hildenbrand Organization: Red Hat Subject: Re: [patch 08/87] proc/vmcore: convert oldmem_pfn_is_ram callback to more generic vmcore callbacks In-Reply-To: <20211110072225.GA18768@MiWiFi-R3L-srv> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 469C5B0000BE X-Stat-Signature: of8ifsihyi3t4sjz1htkjgi1189gq7op Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=DUYl2yEe; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf19.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 216.205.24.124) smtp.mailfrom=david@redhat.com X-HE-Tag: 1636531818-383152 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: On 10.11.21 08:22, Baoquan He wrote: > On 11/08/21 at 06:31pm, Andrew Morton wrote: >> From: David Hildenbrand >> Subject: proc/vmcore: convert oldmem_pfn_is_ram callback to more generic vmcore callbacks >> >> Let's support multiple registered callbacks, making sure that registering >> vmcore callbacks cannot fail. Make the callback return a bool instead of >> an int, handling how to deal with errors internally. Drop unused >> HAVE_OLDMEM_PFN_IS_RAM. >> >> We soon want to make use of this infrastructure from other drivers: >> virtio-mem, registering one callback for each virtio-mem device, to >> prevent reading unplugged virtio-mem memory. >> >> Handle it via a generic vmcore_cb structure, prepared for future >> extensions: for example, once we support virtio-mem on s390x where the >> vmcore is completely constructed in the second kernel, we want to detect >> and add plugged virtio-mem memory ranges to the vmcore in order for them >> to get dumped properly. >> >> Handle corner cases that are unexpected and shouldn't happen in sane >> setups: registering a callback after the vmcore has already been opened >> (warn only) and unregistering a callback after the vmcore has already been >> opened (warn and essentially read only zeroes from that point on). > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > I am fine with the whole patch except of one concern. As above sentence > underscored states, if a callback is unregistered when vmcore has been > opened, it will read out zeros from that point on. And it's done by > judging global variable 'vmcore_cb_unstable' in pfn_is_ram(). This will > cause vmcore dumping in makedumpfile only being able to read out zero > page since then, and may cost long extra time to finish. > > Please see remap_oldmem_pfn_checked(). In makedumpfile, we default to > mmap 4M memory region at one time, then copy out. With this patch, and if > vmcore_cb_unstable is true, kernel will mmap page by page. The extra > time could be huge, e.g on machine with TBs memory, and we only get a > useless vmcore because of loss of core data with high probability. Thanks Baoquan for the quick review! This code is really just to handle the unlikely case of a driver getting unbound from a device that has a callback registered (e.g., a virtio-mem-pci device). Something like this will never happen in practice in a *sane* environment. The only known way I know is if userspace manually unbinds the driver from a virtio-mem-pci device -- which is possible but especially in a kdump environment something without any sane use case. In that case, we'll pr_warn_once("Unexpected vmcore callback unregistration\n"); to let user space know that something weird/unsupported is going on. Long story short: if user space does something nasty, I don't see a problem in some action taking a little longer. > > I am thinking if we can simply panic in the case, since the left dumping > are all zeroed, very likely the vmcore is unavailable any more. IMHO panic() is a little bit too much. Instead of returning zeroes, we could fail the read/mmap operation -- I considered that as an option when I crafted/tested this patch, however, this approach here turned out to be the easiest way to handle something that's really not supported/advised and won't really happen in a sane environment. -- Thanks, David / dhildenb