Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 15 Apr 2020 12:16:18 +0200
From:   Michal Hocko <mhocko@kernel.org>
To:     David Hildenbrand <david@redhat.com>
Cc:     Anshuman Khandual <anshuman.khandual@arm.com>, linux-mm@kvack.org,
        linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org,
        catalin.marinas@arm.com, mark.rutland@arm.com,
        Dan Williams <dan.j.williams@intel.com>,
        Yu Zhao <yuzhao@google.com>,
        Hsin-Yi Wang <hsinyi@chromium.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Steve Capper <steve.capper@arm.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] arm64/hotplug: Process MEM_OFFLINE and
 MEM_CANCEL_OFFLINE events
Message-ID: <20200415101618.GD4629@dhcp22.suse.cz>
References: <1586932784-16315-1-git-send-email-anshuman.khandual@arm.com>
 <31ce355b-abf7-ac3b-a5b4-ae1b0a52fb3c@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <31ce355b-abf7-ac3b-a5b4-ae1b0a52fb3c@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed 15-04-20 09:35:33, David Hildenbrand wrote:
> On 15.04.20 08:39, Anshuman Khandual wrote:
> > This series improves arm64 memory event notifier (hot remove) robustness by
> > enabling it to detect potential problems (if any) in the future. But first
> > it enumerates memory isolation failure reasons that can be sent across a
> > notifier. This series does not go beyond arm64 to explore if these failure
> > reason codes could be used in other existing registered memory notifiers.
> > Please do let me know if there is any other potential use cases, will be
> > happy to incorporate next time around. Also should we add similar failure
> > reasons for online_pages() as well ?
> > 
> > This series has been tested on arm64, boot tested on x86 and build tested
> > on multiple other platforms.
> > 
> 
> I'm sorry, but I have to nack this series. Why?
> 
> 1. A hotplug notifier should not have to bother why offlining failed. He
> received a MEM_GOING_OFFLINE, followed by a MEM_CANCEL_OFFLINE. That's
> all he really has to know. Undo what you've done, end of story.
> 
> 2. Patch 2 just introduces dead code that should never happen unless
> something is horribly broken in the core (memory offlined although
> nacked from notifier). And, it (for *whatever reason*) thinks it's okay
> to bail out if another noYtifier canceled offlining hotplugged memory.
> 
> I fail to see the benefit for core changes and

Agreed! If arm64 wants to check and report early bootmem memory
offlining then just do it. There is no reason to add a whole machinery
for that. Cancel notifier is indeed only supposed to restore the state
before GOING_OFFLINE.

>  4 files changed, 99 insertions(+), 13 deletions(-)

-- 
Michal Hocko
SUSE Labs