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=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED 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 151F1C282DA for ; Wed, 17 Apr 2019 13:10:46 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 936D12173C for ; Wed, 17 Apr 2019 13:10:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 936D12173C 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 EA0A66B0005; Wed, 17 Apr 2019 09:10:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E514D6B0006; Wed, 17 Apr 2019 09:10:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CF32E6B0007; Wed, 17 Apr 2019 09:10:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by kanga.kvack.org (Postfix) with ESMTP id AB00A6B0005 for ; Wed, 17 Apr 2019 09:10:44 -0400 (EDT) Received: by mail-qk1-f199.google.com with SMTP id p3so2755046qkj.18 for ; Wed, 17 Apr 2019 06:10:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-original-authentication-results:x-gm-message-state:subject:to:cc :references:from:openpgp:autocrypt:organization:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=3BBemHHos/Az8olKGWSvDPAkuUx18ExDwYOEbce0Yq4=; b=ism8Syrklt+zQw0srnH3ZKTvdcgmt7Hkeco78Qhm5QXqzJ87s29P7AjejyGRD0sE8Z HJwC2QvfOVGcJdvmBANkcRPCn9RfEiRmQT3FqtU+ZsaiRWsnl+AtX525YmiZNBTtCepv TZc626CvmTUrUmyOBKNeZAhuuRz1GZhHm93diGpYDObP1/5o2EBAFL/2rFk1oluFbs5n oGf5HngWIz7XjJZzsp7O5/459wNPfIF0GCDO1VnnGpQA1mxWv98+Tf17lcY8aVPCNHlR jMq7LD8QANHH4fczHT8ipCke1XDr9vopwneZ18KL9DYT0L+mS3kSPgvGJ2jtSypsLct5 qiYg== X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of david@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com X-Gm-Message-State: APjAAAXhkwVNEcMWgBUVI1dYL1IoB+C0z4aczR8c84FfmSn6nGoKqWZQ JKnSKl1eqcM07Ho0hxcQAXGE4HOGGOcOxnDFzG/xL/3l0jv9pfSKsA+NYMsSVt8pRHfEhjpINWO EjyNcQ3ZXTvjmYQehN2/x207M8hn1P84gALnxgzAou+g/X9SY9206R3r4I2FF4kgiYg== X-Received: by 2002:a05:620a:1659:: with SMTP id c25mr32887571qko.44.1555506644370; Wed, 17 Apr 2019 06:10:44 -0700 (PDT) X-Google-Smtp-Source: APXvYqxaDK/CjI3dhAcFHN/izRsswbfc6Ygi4okP4gCUpYEunTmrKuIZju9qs6buJffug66iXlCK X-Received: by 2002:a05:620a:1659:: with SMTP id c25mr32887510qko.44.1555506643588; Wed, 17 Apr 2019 06:10:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1555506643; cv=none; d=google.com; s=arc-20160816; b=vWFHIZ74ol7rC1d/YkWsF17O0nNMRin2XBRMRCNvlToeSlNvG1oWBUuOTiqxI3NRch mTl5+d/ailwCtoopcByfNh+O7eHHJS5SGHukYcUq3FdXWYPYHXxV3llsaPXBK8Q3nIuT HDrWnTsDtjhwxxMyKv+twc79X1svFFdkhscNaJa+E4NeWfSW51QVEZjVHYKx8ZdgPyGb Tn7p0Yyefb5kP1lspAIoG2Zs0eyGRN6X0NH2Ch/V+La/YQuayF7wbyhMe5xxbWknYuKG E5JUXnZNLKowzOd72T5b0sPqpI0ngoBikTuS6qBCOOHSYhpn8cZje67KKggjTLQ116BR mswQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:organization:autocrypt:openpgp:from :references:cc:to:subject; bh=3BBemHHos/Az8olKGWSvDPAkuUx18ExDwYOEbce0Yq4=; b=phPgqtIggJF4W8sxqsd25eNXsye91cOudflSawsZlAnaDvfT+FqwM1uOKjKAzFJRr+ /tv99ffhYFuXv1Kb+4ztvPlK7emrwIOUas+FLYmSEaV3mkkrBTFDIMIRp3aa30teSGh3 enOd+/yGqbwYeoKX3D4H7Q3/DtwB4W9gknrZt9J6U/G/k5ZIGAatCYVsZYM6ZVbzq0KV 1zI8rEDnE+65hmPYGIDH8cSFZAfwVnG/ivunUnEpf6ZL2JX+c+1wgCaAfxnT7l9dP8A6 yRCmxpImkAal7WJezCJbOmsbSjPCDXHIThq2w/lh9NZOTBmTdOPRjzJFhImWp8/056MG 1RzA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of david@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from mx1.redhat.com (mx1.redhat.com. [209.132.183.28]) by mx.google.com with ESMTPS id z189si14431869qkd.76.2019.04.17.06.10.43 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 17 Apr 2019 06:10:43 -0700 (PDT) Received-SPF: pass (google.com: domain of david@redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; Authentication-Results: mx.google.com; spf=pass (google.com: domain of david@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6820830842AC; Wed, 17 Apr 2019 13:10:42 +0000 (UTC) Received: from [10.36.116.187] (ovpn-116-187.ams2.redhat.com [10.36.116.187]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2A5355D9E1; Wed, 17 Apr 2019 13:10:39 +0000 (UTC) Subject: Re: [PATCH v1 2/4] mm/memory_hotplug: Make unregister_memory_section() never fail To: Oscar Salvador , linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman , "Rafael J. Wysocki" , Ingo Molnar , Andrew Banman , "mike.travis@hpe.com" , Andrew Morton , Michal Hocko , Pavel Tatashin , Qian Cai , Wei Yang , Arun KS , Mathieu Malaterre References: <20190409100148.24703-1-david@redhat.com> <20190409100148.24703-3-david@redhat.com> <1555505146.3139.26.camel@suse.de> From: David Hildenbrand Openpgp: preference=signencrypt Autocrypt: addr=david@redhat.com; prefer-encrypt=mutual; keydata= xsFNBFXLn5EBEAC+zYvAFJxCBY9Tr1xZgcESmxVNI/0ffzE/ZQOiHJl6mGkmA1R7/uUpiCjJ dBrn+lhhOYjjNefFQou6478faXE6o2AhmebqT4KiQoUQFV4R7y1KMEKoSyy8hQaK1umALTdL QZLQMzNE74ap+GDK0wnacPQFpcG1AE9RMq3aeErY5tujekBS32jfC/7AnH7I0v1v1TbbK3Gp XNeiN4QroO+5qaSr0ID2sz5jtBLRb15RMre27E1ImpaIv2Jw8NJgW0k/D1RyKCwaTsgRdwuK Kx/Y91XuSBdz0uOyU/S8kM1+ag0wvsGlpBVxRR/xw/E8M7TEwuCZQArqqTCmkG6HGcXFT0V9 PXFNNgV5jXMQRwU0O/ztJIQqsE5LsUomE//bLwzj9IVsaQpKDqW6TAPjcdBDPLHvriq7kGjt WhVhdl0qEYB8lkBEU7V2Yb+SYhmhpDrti9Fq1EsmhiHSkxJcGREoMK/63r9WLZYI3+4W2rAc UucZa4OT27U5ZISjNg3Ev0rxU5UH2/pT4wJCfxwocmqaRr6UYmrtZmND89X0KigoFD/XSeVv jwBRNjPAubK9/k5NoRrYqztM9W6sJqrH8+UWZ1Idd/DdmogJh0gNC0+N42Za9yBRURfIdKSb B3JfpUqcWwE7vUaYrHG1nw54pLUoPG6sAA7Mehl3nd4pZUALHwARAQABzSREYXZpZCBIaWxk ZW5icmFuZCA8ZGF2aWRAcmVkaGF0LmNvbT7CwX4EEwECACgFAljj9eoCGwMFCQlmAYAGCwkI BwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJEE3eEPcA/4Na5IIP/3T/FIQMxIfNzZshIq687qgG 8UbspuE/YSUDdv7r5szYTK6KPTlqN8NAcSfheywbuYD9A4ZeSBWD3/NAVUdrCaRP2IvFyELj xoMvfJccbq45BxzgEspg/bVahNbyuBpLBVjVWwRtFCUEXkyazksSv8pdTMAs9IucChvFmmq3 jJ2vlaz9lYt/lxN246fIVceckPMiUveimngvXZw21VOAhfQ+/sofXF8JCFv2mFcBDoa7eYob s0FLpmqFaeNRHAlzMWgSsP80qx5nWWEvRLdKWi533N2vC/EyunN3HcBwVrXH4hxRBMco3jvM m8VKLKao9wKj82qSivUnkPIwsAGNPdFoPbgghCQiBjBe6A75Z2xHFrzo7t1jg7nQfIyNC7ez MZBJ59sqA9EDMEJPlLNIeJmqslXPjmMFnE7Mby/+335WJYDulsRybN+W5rLT5aMvhC6x6POK z55fMNKrMASCzBJum2Fwjf/VnuGRYkhKCqqZ8gJ3OvmR50tInDV2jZ1DQgc3i550T5JDpToh dPBxZocIhzg+MBSRDXcJmHOx/7nQm3iQ6iLuwmXsRC6f5FbFefk9EjuTKcLMvBsEx+2DEx0E UnmJ4hVg7u1PQ+2Oy+Lh/opK/BDiqlQ8Pz2jiXv5xkECvr/3Sv59hlOCZMOaiLTTjtOIU7Tq 7ut6OL64oAq+zsFNBFXLn5EBEADn1959INH2cwYJv0tsxf5MUCghCj/CA/lc/LMthqQ773ga uB9mN+F1rE9cyyXb6jyOGn+GUjMbnq1o121Vm0+neKHUCBtHyseBfDXHA6m4B3mUTWo13nid 0e4AM71r0DS8+KYh6zvweLX/LL5kQS9GQeT+QNroXcC1NzWbitts6TZ+IrPOwT1hfB4WNC+X 2n4AzDqp3+ILiVST2DT4VBc11Gz6jijpC/KI5Al8ZDhRwG47LUiuQmt3yqrmN63V9wzaPhC+ xbwIsNZlLUvuRnmBPkTJwwrFRZvwu5GPHNndBjVpAfaSTOfppyKBTccu2AXJXWAE1Xjh6GOC 8mlFjZwLxWFqdPHR1n2aPVgoiTLk34LR/bXO+e0GpzFXT7enwyvFFFyAS0Nk1q/7EChPcbRb hJqEBpRNZemxmg55zC3GLvgLKd5A09MOM2BrMea+l0FUR+PuTenh2YmnmLRTro6eZ/qYwWkC u8FFIw4pT0OUDMyLgi+GI1aMpVogTZJ70FgV0pUAlpmrzk/bLbRkF3TwgucpyPtcpmQtTkWS gDS50QG9DR/1As3LLLcNkwJBZzBG6PWbvcOyrwMQUF1nl4SSPV0LLH63+BrrHasfJzxKXzqg rW28CTAE2x8qi7e/6M/+XXhrsMYG+uaViM7n2je3qKe7ofum3s4vq7oFCPsOgwARAQABwsFl BBgBAgAPBQJVy5+RAhsMBQkJZgGAAAoJEE3eEPcA/4NagOsP/jPoIBb/iXVbM+fmSHOjEshl KMwEl/m5iLj3iHnHPVLBUWrXPdS7iQijJA/VLxjnFknhaS60hkUNWexDMxVVP/6lbOrs4bDZ NEWDMktAeqJaFtxackPszlcpRVkAs6Msn9tu8hlvB517pyUgvuD7ZS9gGOMmYwFQDyytpepo YApVV00P0u3AaE0Cj/o71STqGJKZxcVhPaZ+LR+UCBZOyKfEyq+ZN311VpOJZ1IvTExf+S/5 lqnciDtbO3I4Wq0ArLX1gs1q1XlXLaVaA3yVqeC8E7kOchDNinD3hJS4OX0e1gdsx/e6COvy qNg5aL5n0Kl4fcVqM0LdIhsubVs4eiNCa5XMSYpXmVi3HAuFyg9dN+x8thSwI836FoMASwOl C7tHsTjnSGufB+D7F7ZBT61BffNBBIm1KdMxcxqLUVXpBQHHlGkbwI+3Ye+nE6HmZH7IwLwV W+Ajl7oYF+jeKaH4DZFtgLYGLtZ1LDwKPjX7VAsa4Yx7S5+EBAaZGxK510MjIx6SGrZWBrrV TEvdV00F2MnQoeXKzD7O4WFbL55hhyGgfWTHwZ457iN9SgYi1JLPqWkZB0JRXIEtjd4JEQcx +8Umfre0Xt4713VxMygW0PnQt5aSQdMD58jHFxTk092mU+yIHj5LeYgvwSgZN4airXk5yRXl SE+xAvmumFBY Organization: Red Hat GmbH Message-ID: Date: Wed, 17 Apr 2019 15:10:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <1555505146.3139.26.camel@suse.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Wed, 17 Apr 2019 13:10:42 +0000 (UTC) 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 17.04.19 14:45, Oscar Salvador wrote: > On Tue, 2019-04-09 at 12:01 +0200, David Hildenbrand wrote: >> Failing while removing memory is mostly ignored and cannot really be >> handled. Let's treat errors in unregister_memory_section() in a nice >> way, warning, but continuing. >> >> Cc: Greg Kroah-Hartman >> Cc: "Rafael J. Wysocki" >> Cc: Ingo Molnar >> Cc: Andrew Banman >> Cc: "mike.travis@hpe.com" >> Cc: David Hildenbrand >> Cc: Andrew Morton >> Cc: Oscar Salvador >> Cc: Michal Hocko >> Cc: Pavel Tatashin >> Cc: Qian Cai >> Cc: Wei Yang >> Cc: Arun KS >> Cc: Mathieu Malaterre >> Signed-off-by: David Hildenbrand >> --- >> drivers/base/memory.c | 16 +++++----------- >> include/linux/memory.h | 2 +- >> mm/memory_hotplug.c | 4 +--- >> 3 files changed, 7 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/base/memory.c b/drivers/base/memory.c >> index 0c9e22ffa47a..f180427e48f4 100644 >> --- a/drivers/base/memory.c >> +++ b/drivers/base/memory.c >> @@ -734,15 +734,18 @@ unregister_memory(struct memory_block *memory) >> { >> BUG_ON(memory->dev.bus != &memory_subsys); >> >> - /* drop the ref. we got in remove_memory_section() */ >> + /* drop the ref. we got via find_memory_block() */ >> put_device(&memory->dev); >> device_unregister(&memory->dev); >> } >> >> -static int remove_memory_section(struct mem_section *section) >> +void unregister_memory_section(struct mem_section *section) >> { >> struct memory_block *mem; >> >> + if (WARN_ON_ONCE(!present_section(section))) >> + return; >> + >> mutex_lock(&mem_sysfs_mutex); >> >> /* >> @@ -763,15 +766,6 @@ static int remove_memory_section(struct >> mem_section *section) >> >> out_unlock: >> mutex_unlock(&mem_sysfs_mutex); >> - return 0; >> -} >> - >> -int unregister_memory_section(struct mem_section *section) >> -{ >> - if (!present_section(section)) >> - return -EINVAL; >> - >> - return remove_memory_section(section); >> } >> #endif /* CONFIG_MEMORY_HOTREMOVE */ >> >> diff --git a/include/linux/memory.h b/include/linux/memory.h >> index a6ddefc60517..e1dc1bb2b787 100644 >> --- a/include/linux/memory.h >> +++ b/include/linux/memory.h >> @@ -113,7 +113,7 @@ extern int >> register_memory_isolate_notifier(struct notifier_block *nb); >> extern void unregister_memory_isolate_notifier(struct notifier_block >> *nb); >> int hotplug_memory_register(int nid, struct mem_section *section); >> #ifdef CONFIG_MEMORY_HOTREMOVE >> -extern int unregister_memory_section(struct mem_section *); >> +extern void unregister_memory_section(struct mem_section *); >> #endif >> extern int memory_dev_init(void); >> extern int memory_notify(unsigned long val, void *v); >> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >> index 696ed7ee5e28..b0cb05748f99 100644 >> --- a/mm/memory_hotplug.c >> +++ b/mm/memory_hotplug.c >> @@ -527,9 +527,7 @@ static int __remove_section(struct zone *zone, >> struct mem_section *ms, >> if (!valid_section(ms)) >> return ret; >> >> - ret = unregister_memory_section(ms); >> - if (ret) >> - return ret; >> + unregister_memory_section(ms); > > So, technically unregister_memory_section() can __only__ fail in case > the section is not present, returning -EINVAL. > > Now, I was checking how the pair valid/present sections work. > Unless I am mistaken, we only mark sections as memmap those sections > that are present. > > This can come from two paths: > > - Early boot: > * memblocks_present > memory_present - mark sections as present > sparse_init - iterates only over present sections > sparse_init_nid > sparse_init_one_section - mark section as valid > > - Hotplug path: > * sparse_add_one_section > section_mark_present - mark section as present > sparse_init_one_section - mark section as valid > > > During early boot, sparse_init iterates __only__ over present sections, > so only those are marked valid as well, and during hotplug, the section > is both marked present and valid. > > All in all, I think that we are safe if we remove the present_section > check in your new unregister_memory_section(), as a valid_section > cannot be non-present, and we do already catch those early in > __remove_section(). Yes, dropping the check completely would be the next step. > > Then, the only thing missing to be completely error-less in that > codepath is to make unregister_mem_sect_under_nodes() void return-type. > Not that it matters a lot as we are already ignoring its return code, > but I find that quite disturbing and wrong. > > So, would you like to take this patch in your patchset in case you re- > submit? I already had a look at that approach but would like to defer it, until I eventually factor out creation/deletion of memory devices (see my other RFC, still some work has to be done). Reasons: 1. We only support unplug of system ram memory belonging to exactly one node. No need to iterate over all nodes. mem->node should later tell us exactly what needs to be removed. No need to iterate at all. 2. unregister_mem_sect_under_nodes() should be replaced by "unregister_mem_block_under_nodes()". We only support unplug of whole memory blocks. Ripping out random sections from a memory block is not going to work and is not supported. IOW, this is some leftover when people turned memory block devices to span multiple sections. Once we clean that up, this problem goes away. -- Thanks, David / dhildenb