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.5 required=3.0 tests=MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,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 D9506C33CB1 for ; Fri, 17 Jan 2020 14:52:37 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9945E2083E for ; Fri, 17 Jan 2020 14:52:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9945E2083E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 45E9C6B04A9; Fri, 17 Jan 2020 09:52:37 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3E8EA6B04AA; Fri, 17 Jan 2020 09:52:37 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2B1416B04AB; Fri, 17 Jan 2020 09:52:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0151.hostedemail.com [216.40.44.151]) by kanga.kvack.org (Postfix) with ESMTP id 0E7996B04A9 for ; Fri, 17 Jan 2020 09:52:37 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with SMTP id BD7E7180AD817 for ; Fri, 17 Jan 2020 14:52:36 +0000 (UTC) X-FDA: 76387417512.08.coal90_6e0ca6ef28806 X-HE-Tag: coal90_6e0ca6ef28806 X-Filterd-Recvd-Size: 6025 Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by imf48.hostedemail.com (Postfix) with ESMTP for ; Fri, 17 Jan 2020 14:52:36 +0000 (UTC) Received: by mail-wm1-f67.google.com with SMTP id 20so7916072wmj.4 for ; Fri, 17 Jan 2020 06:52:36 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=AkURUQJ+DBuKMDSx+itAbwtb7hBHyuXsOO2uXs2DdgU=; b=XG8e/QhuHVAnire80R1yhJynVmcEZPNhu6drPkDoY1yLqgFrk0tOOysrZQzaY3b6pd MAyfTz3o8lNLNaZTSUGa2J+WZNxGU/kXPc5MmojuN+vAU0bAyC5oio7Lycd+ppDcQ44b 0+KsapTH69fAEvJ3R9kk6ibm5EjwHKRrk75J4SkBR/xilNk09qw1uVG53s9E8k2nlqw3 zPOarMNMUay19U3SSlOoU3XZh7bLlVoZL2l2gxVAiUmrAtSmF5+ChYtfpElHvhU2KGU/ SVmRmJ28gZUF3EyxxtlySjUILzArUisrlg3L25qd5wlIoHvN0D32MXBvNXtIvZPJ65mk 6vag== X-Gm-Message-State: APjAAAV3gIN/KOtpt1UAkRig6vepyWy8EPHgTteDlIa6/KZXxsP3Mm0D whgGejT1DLXtbDemJJDUAtU= X-Google-Smtp-Source: APXvYqx6QdFP6CoZLoNRt4b0Ti6hyVkI0oAXyG8bSOgUtIHhWTQMk4sw68ciudP9UEYs0HOMMg2GPg== X-Received: by 2002:a05:600c:2290:: with SMTP id 16mr5039901wmf.93.1579272755115; Fri, 17 Jan 2020 06:52:35 -0800 (PST) Received: from localhost (prg-ext-pat.suse.com. [213.151.95.130]) by smtp.gmail.com with ESMTPSA id n8sm34315771wrx.42.2020.01.17.06.52.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Jan 2020 06:52:34 -0800 (PST) Date: Fri, 17 Jan 2020 15:52:33 +0100 From: Michal Hocko To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Greg Kroah-Hartman , "Rafael J. Wysocki" , Andrew Morton , Leonardo Bras , Nathan Lynch , Allison Randal , Nathan Fontenot , Thomas Gleixner , Dan Williams , Stephen Rothwell , Anshuman Khandual , lantianyu1986@gmail.com, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH RFC v1] mm: is_mem_section_removable() overhaul Message-ID: <20200117145233.GB19428@dhcp22.suse.cz> References: <20200117105759.27905-1-david@redhat.com> <20200117113353.GT19428@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.2 (2019-09-21) 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 Fri 17-01-20 14:08:06, David Hildenbrand wrote: > On 17.01.20 12:33, Michal Hocko wrote: > > On Fri 17-01-20 11:57:59, David Hildenbrand wrote: > >> Let's refactor that code. We want to check if we can offline memory > >> blocks. Add a new function is_mem_section_offlineable() for that and > >> make it call is_mem_section_offlineable() for each contained section. > >> Within is_mem_section_offlineable(), add some more sanity checks and > >> directly bail out if the section contains holes or if it spans multiple > >> zones. > > > > I didn't read the patch (yet) but I am wondering. If we want to touch > > this code, can we simply always return true there? I mean whoever > > depends on this check is racy and the failure can happen even after > > the sysfs says good to go, right? The check is essentially as expensive > > as calling the offlining code itself. So the only usecase I can think of > > is a dumb driver to crawl over blocks and check which is removable and > > try to hotremove it. But just trying to offline one block after another > > is essentially going to achieve the same. > > Some thoughts: > > 1. It allows you to check if memory is likely to be offlineable without > doing expensive locking and trying to isolate pages (meaning: > zone->lock, mem_hotplug_lock. but also, calling drain_all_pages() > when isolating) > > 2. There are use cases that want to identify a memory block/DIMM to > unplug. One example is PPC DLPAR code (see this patch). Going over all > memory block trying to offline them is an expensive operation. > > 3. powerpc-utils (https://github.com/ibm-power-utilities/powerpc-utils) > makes use of /sys/.../removable to speed up the search AFAIK. Well, while I do see those points I am not really sure they are worth having a broken (by-definition) interface. > 4. lsmem displays/groups by "removable". Is anybody really using that? > 5. If "removable=false" then it usually really is not offlineable. > Of course, there could also be races (free the last unmovable page), > but it means "don't even try". OTOH, "removable=true" is more racy, > and gives less guarantees. ("looks okay, feel free to try") Yeah, but you could be already pessimistic and try movable zones before other kernel zones. > > Or does anybody see any reasonable usecase that would break if we did > > that unconditional behavior? > > If we would return always "true", then the whole reason the > interface originally was introduced would be "broken" (meaning, less > performant as you would try to offline any memory block). I would argue that the whole interface is broken ;). Not the first time in the kernel development history and not the last time either. What I am trying to say here is that unless there are _real_ usecases depending on knowing that something surely is _not_ offlineable then I would just try to drop the functionality while preserving the interface and see what happens. -- Michal Hocko SUSE Labs