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=-2.5 required=3.0 tests=MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 1718BC432C1 for ; Tue, 24 Sep 2019 11:47:29 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D5F092146E for ; Tue, 24 Sep 2019 11:47:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D5F092146E 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 6D50C6B0005; Tue, 24 Sep 2019 07:47:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 687226B000A; Tue, 24 Sep 2019 07:47:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5C55C6B000C; Tue, 24 Sep 2019 07:47:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0063.hostedemail.com [216.40.44.63]) by kanga.kvack.org (Postfix) with ESMTP id 3D8D16B0005 for ; Tue, 24 Sep 2019 07:47:28 -0400 (EDT) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id D2F9D52C8 for ; Tue, 24 Sep 2019 11:47:27 +0000 (UTC) X-FDA: 75969638934.13.night41_5e397c478db44 X-HE-Tag: night41_5e397c478db44 X-Filterd-Recvd-Size: 4189 Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by imf16.hostedemail.com (Postfix) with ESMTP for ; Tue, 24 Sep 2019 11:47:27 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id AA33AB681; Tue, 24 Sep 2019 11:47:25 +0000 (UTC) Date: Tue, 24 Sep 2019 13:47:25 +0200 From: Michal Hocko To: Anshuman Khandual Cc: David Hildenbrand , linux-mm@kvack.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, Oscar Salvador , Pavel Tatashin , Dan Williams Subject: Re: [PATCH] mm/hotplug: Reorder memblock_[free|remove]() calls in try_remove_memory() Message-ID: <20190924114725.GL23050@dhcp22.suse.cz> References: <1568612857-10395-1-git-send-email-anshuman.khandual@arm.com> <20190923105224.GH6016@dhcp22.suse.cz> <9b85f517-fee5-650a-4e18-29408ca85804@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) 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 Tue 24-09-19 09:42:31, Anshuman Khandual wrote: > > > On 09/23/2019 04:24 PM, David Hildenbrand wrote: > > On 23.09.19 12:52, Michal Hocko wrote: > >> On Mon 16-09-19 11:17:37, Anshuman Khandual wrote: > >>> In add_memory_resource() the memory range to be hot added first gets into > >>> the memblock via memblock_add() before arch_add_memory() is called on it. > >>> Reverse sequence should be followed during memory hot removal which already > >>> is being followed in add_memory_resource() error path. This now ensures > >>> required re-order between memblock_[free|remove]() and arch_remove_memory() > >>> during memory hot-remove. > >> > >> This changelog is not really easy to follow. First of all please make > >> sure to explain whether there is any actual problem to solve or this is > >> an aesthetic matter. Please think of people reading this changelog in > >> few years and scratching their heads what you were thinking back then... > >> > > > > I think it would make sense to just draft the current call sequence in > > the add and the removal path (instead of describing it) - then it > > becomes obvious why this is a cosmetic change. > > Does this look okay ? > > mm/hotplug: Reorder memblock_[free|remove]() calls in try_remove_memory() > > Currently during memory hot add procedure, memory gets into memblock before > calling arch_add_memory() which creates it's linear mapping. > > add_memory_resource() { > .................. > memblock_add_node() > .................. > arch_add_memory() > .................. > } > > But during memory hot remove procedure, removal from memblock happens first > before it's linear mapping gets teared down with arch_remove_memory() which > is not coherent. Resource removal should happen in reverse order as they > were added. > > try_remove_memory() { > .................. > memblock_free() > memblock_remove() > .................. > arch_remove_memory() > .................. > } > > This changes the sequence of resource removal including memblock and linear > mapping tear down during memory hot remove which will now be the reverse > order in which they were added during memory hot add. The changed removal > order looks like the following. > > try_remove_memory() { > .................. > arch_remove_memory() > .................. > memblock_free() > memblock_remove() > .................. > } OK, this is better. I would just a note that the inconsistency doesn't pose any problem now but if somebody makes any assumptions about linear mappings then it could get subtly broken like your example for arm64 which has found a different solution in the meantime. -- Michal Hocko SUSE Labs