From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754016AbcHZTh6 (ORCPT ); Fri, 26 Aug 2016 15:37:58 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:32052 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751139AbcHZTh4 (ORCPT ); Fri, 26 Aug 2016 15:37:56 -0400 Subject: Re: [PATCH 1/2] xen/x86: Convert to hotplug state machine To: Sebastian Andrzej Siewior References: <1471272407-4292-1-git-send-email-boris.ostrovsky@oracle.com> <1471272407-4292-2-git-send-email-boris.ostrovsky@oracle.com> <20160817083309.fmkksdchqexaxncz@linutronix.de> Cc: david.vrabel@citrix.com, jgross@suse.com, xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org From: Boris Ostrovsky Message-ID: Date: Fri, 26 Aug 2016 15:37:38 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160817083309.fmkksdchqexaxncz@linutronix.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/17/2016 04:33 AM, Sebastian Andrzej Siewior wrote: > On 2016-08-15 10:46:46 [-0400], Boris Ostrovsky wrote: >> Switch to new CPU hotplug infrastructure. >> >> Signed-off-by: Boris Ostrovsky >> Suggested-by: Sebastian Andrzej Siewior >> --- >> arch/x86/xen/enlighten.c | 115 +++++++++++++++++++++++++------------------- >> include/linux/cpuhotplug.h | 2 + >> 2 files changed, 67 insertions(+), 50 deletions(-) >> >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >> index c7f6b1f..2283976 100644 >> --- a/arch/x86/xen/enlighten.c >> +++ b/arch/x86/xen/enlighten.c >> @@ -1541,6 +1543,24 @@ static void __init xen_dom0_set_legacy_features(void) >> x86_platform.legacy.rtc = 1; >> } >> >> +static int xen_cpuhp_setup(void) >> +{ >> + int rc; >> + >> + rc = cpuhp_setup_state_nocalls(CPUHP_XEN_PREPARE, >> + "XEN_HVM_GUEST_PREPARE", >> + xen_cpu_up_prepare, xen_cpu_up_cancel); > The old states UP_CANCEL is different from UP_PREPARE. The latter was > invoked only in the error case while your new callback is always > invoked. From looking at the code you free memory in > xen_cpu_up_cancel() which was allocated in xen_cpu_up_prepare() and > therefore I would name it xen_cpu_dead(). Yes, "cancel" is wrong term to use here. > If you do find the time, you might manage to rework the code to avoid > using the _nocalls() function. If see this right, you use > xen_setup_vcpu_info_placement() for the init in the first place. This > uses for_each_possible_cpu macro. The cpuhp_setup_state() function would > perform the init for all CPUs before they come up. I am not sure I see what this would buy us. Besides, cpuhp_setup_state() uses for_each_present_cpu(). > >> + if (!rc) { >> + rc = cpuhp_setup_state_nocalls(CPUHP_AP_XEN_ONLINE, > If there is no need to run this after KVM's CLK callback please use > CPUHP_AP_ONLINE_DYN. If there is such a need then please document it. OK. -boris > >> + "XEN_HVM_GUEST_PREPARE", >> + xen_cpu_up_online, NULL); >> + if (rc) >> + cpuhp_remove_state_nocalls(CPUHP_XEN_PREPARE); >> + } >> + >> + return rc; >> +} >> + >> /* First C function to be called on Xen boot */ >> asmlinkage __visible void __init xen_start_kernel(void) >> { > Sebastian