From patchwork Fri Jan 24 15:53:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Hildenbrand X-Patchwork-Id: 11350617 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 71A3C921 for ; Fri, 24 Jan 2020 15:53:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 34D4320709 for ; Fri, 24 Jan 2020 15:53:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="JNPgVKiR" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 34D4320709 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 5CA926B0005; Fri, 24 Jan 2020 10:53:51 -0500 (EST) Delivered-To: linux-mm-outgoing@kvack.org Received: by kanga.kvack.org (Postfix, from userid 40) id 579E86B0006; Fri, 24 Jan 2020 10:53:51 -0500 (EST) X-Original-To: int-list-linux-mm@kvack.org X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 442C16B0007; Fri, 24 Jan 2020 10:53:51 -0500 (EST) X-Original-To: linux-mm@kvack.org X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0110.hostedemail.com [216.40.44.110]) by kanga.kvack.org (Postfix) with ESMTP id 2BFC46B0005 for ; Fri, 24 Jan 2020 10:53:51 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with SMTP id D27D0181AEF1A for ; Fri, 24 Jan 2020 15:53:50 +0000 (UTC) X-FDA: 76412973420.21.goose97_2175b4399900e X-Spam-Summary: 1,0,0,,d41d8cd98f00b204,david@redhat.com,:linux-kernel@vger.kernel.org::david@redhat.com:mhocko@kernel.org:dan.j.williams@intel.com:gregkh@linuxfoundation.org:rafael@kernel.org:akpm@linux-foundation.org:powerpc-utils-devel@googlegroups.com:util-linux@vger.kernel.org:pbadari@us.ibm.com:nfont@linux.vnet.ibm.com:rcj@linux.vnet.ibm.com:heiko.carstens@de.ibm.com:kzak@redhat.com,RULES_HIT:30045:30051:30054:30064:30069:30070:30075,0,RBL:207.211.31.81:@redhat.com:.lbl8.mailshell.net-62.18.0.100 66.10.201.10,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:ft,MSBL:0,DNSBL:neutral,Custom_rules:0:0:0,LFtime:25,LUA_SUMMARY:none X-HE-Tag: goose97_2175b4399900e X-Filterd-Recvd-Size: 7628 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by imf43.hostedemail.com (Postfix) with ESMTP for ; Fri, 24 Jan 2020 15:53:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579881229; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=0ihvg6/8J0yB5dGMbm8QQOnnlGHafa6tG6Y9MDBJXnM=; b=JNPgVKiR+Z1/iosI5BlmoosI+YYG18oYcKqCIlDYBeZFerpCbUHPSSVtn8C/yLFHMRRQWQ gesCk/QELZH21Dqh6SQVGGNotKaAfIpIspwOmwg2VLOeutpSW8SxZ7ASDtsU0/nXOTiiVz GqqZRsK3uwwa4ynL7WBrnslpdaHrlr4= 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-181-y7cVVPsBM9y2a8GheaL2jg-1; Fri, 24 Jan 2020 10:53:46 -0500 X-MC-Unique: y7cVVPsBM9y2a8GheaL2jg-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2624D195A76F; Fri, 24 Jan 2020 15:53:44 +0000 (UTC) Received: from t480s.redhat.com (ovpn-116-39.ams2.redhat.com [10.36.116.39]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8F9F95DA2C; Fri, 24 Jan 2020 15:53:37 +0000 (UTC) From: David Hildenbrand To: linux-kernel@vger.kernel.org Cc: linux-mm@kvack.org, David Hildenbrand , Michal Hocko , Dan Williams , Greg Kroah-Hartman , "Rafael J. Wysocki" , Andrew Morton , powerpc-utils-devel@googlegroups.com, util-linux@vger.kernel.org, Badari Pulavarty , Nathan Fontenot , Robert Jennings , Heiko Carstens , Karel Zak Subject: [PATCH RFC] drivers/base/memory.c: indicate all memory blocks as removable Date: Fri, 24 Jan 2020 16:53:36 +0100 Message-Id: <20200124155336.17126-1-david@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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: We see multiple issues with the implementation/interface to compute whether a memory block can be offlined (exposed via /sys/devices/system/memory/memoryX/removable) and would like to simplify it (remove the implementation). 1. It runs basically lockless. While this might be good for performance, we see possible races with memory offlining/unplug that will require at least some sort of locking to fix. 2. Nowadays, more false positives are possible. No arch-specific checks are performed that validate if memory offlining will not be denied right away (and such check will require locking). For example, arm64 won't allow to offline any memory block that was added during boot - which will imply a very high error rate. Other archs have other constraints. 3. The interface is inherently racy. E.g., if a memory block is detected to be removable (and was not a false positive at that time), there is still no guarantee that offlining will actually succeed. So any caller already has to deal with false positives. 4. It is unclear which performance benefit this interface actually provides. The introducing commit 5c755e9fd813 ("memory-hotplug: add sysfs removable attribute for hotplug memory remove") mentioned "A user-level agent must be able to identify which sections of memory are likely to be removable before attempting the potentially expensive operation." However, no actual performance comparison was included. Known users: - lsmem: Will group memory blocks based on the "removable" property. [1] - chmem: Indirect user. It has a RANGE mode where one can specify removable ranges identified via lsmem to be offlined. However, it also has a "SIZE" mode, which allows a sysadmin to skip the manual "identify removable blocks" step. [2] - powerpc-utils: Uses the "removable" attribute to skip some memory blocks right away when trying to find some to offline+remove. However, with ballooning enabled, it already skips this information completely (because it once resulted in many false negatives). Therefore, the implementation can deal with false positives properly already. [3] With CONFIG_MEMORY_HOTREMOVE, always indicating "removable" should not break any user space tool. We implement a very bad heuristic now. (in contrast: always returning "not removable" would at least affect powerpc-utils) Without CONFIG_MEMORY_HOTREMOVE we cannot offline anything, so report "not removable" as before. Original discussion can be found in [4] ("[PATCH RFC v1] mm: is_mem_section_removable() overhaul"). Other users of is_mem_section_removable() will be removed next, so that we can remove is_mem_section_removable() completely. [1] http://man7.org/linux/man-pages/man1/lsmem.1.html [2] http://man7.org/linux/man-pages/man8/chmem.8.html [3] https://github.com/ibm-power-utilities/powerpc-utils [4] https://lkml.kernel.org/r/20200117105759.27905-1-david@redhat.com Suggested-by: Michal Hocko Cc: Dan Williams Cc: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" Cc: Andrew Morton Cc: powerpc-utils-devel@googlegroups.com Cc: util-linux@vger.kernel.org Cc: Badari Pulavarty Cc: Nathan Fontenot Cc: Robert Jennings Cc: Heiko Carstens Cc: Karel Zak Signed-off-by: David Hildenbrand Acked-by: Michal Hocko --- 1. Are there any use cases that really require this interface to keep producing "more reliable" results? 2. Is there any real performance advantage when using this interface to identify memory blocks to offline? --- drivers/base/memory.c | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index 6503f5d0b749..d78a92f09984 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -105,30 +105,17 @@ static ssize_t phys_index_show(struct device *dev, } /* - * Show whether the memory block is likely to be offlineable (or is already - * offline). Once offline, the memory block could be removed. The return - * value does, however, not indicate that there is a way to remove the - * memory block. + * Legacy interface that we cannot remove. Always indicate "removable" + * with CONFIG_MEMORY_HOTREMOVE - bad heuristic. */ static ssize_t removable_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct memory_block *mem = to_memory_block(dev); - unsigned long pfn; - int ret = 1, i; - - if (mem->state != MEM_ONLINE) - goto out; - - for (i = 0; i < sections_per_block; i++) { - if (!present_section_nr(mem->start_section_nr + i)) - continue; - pfn = section_nr_to_pfn(mem->start_section_nr + i); - ret &= is_mem_section_removable(pfn, PAGES_PER_SECTION); - } - -out: - return sprintf(buf, "%d\n", ret); +#ifdef CONFIG_MEMORY_HOTREMOVE + return sprintf(buf, "1\n"); +#else + return sprintf(buf, "0\n"); +#endif } /*