Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753372AbcCYPXW (ORCPT ); Fri, 25 Mar 2016 11:23:22 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:51836 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751857AbcCYPXV (ORCPT ); Fri, 25 Mar 2016 11:23:21 -0400 Subject: Re: [PATCH 1/2] xen/x86: Move irq allocation from Xen smp_op.cpu_up() To: Konrad Rzeszutek Wilk References: <1458221613-21563-1-git-send-email-boris.ostrovsky@oracle.com> <1458221613-21563-2-git-send-email-boris.ostrovsky@oracle.com> <20160325151036.GD17902@char.us.oracle.com> Cc: david.vrabel@citrix.com, tglx@linutronix.de, x86@kernel.org, linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org From: Boris Ostrovsky Message-ID: <56F557D8.80108@oracle.com> Date: Fri, 25 Mar 2016 11:23:04 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160325151036.GD17902@char.us.oracle.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2209 Lines: 79 On 03/25/2016 11:10 AM, Konrad Rzeszutek Wilk wrote: > On Thu, Mar 17, 2016 at 09:33:32AM -0400, Boris Ostrovsky wrote: >> Commit ce0d3c0a6fb1 ("genirq: Revert sparse irq locking around >> __cpu_up() and move it to x86 for now") reverted irq locking >> introduced by commit a89941816726 ("hotplug: Prevent alloc/free >> of irq descriptors during cpu up/down") because of Xen allocating >> irqs in both of its cpu_up ops. >> >> We can move those allocations into CPU notifiers so that original >> patch can be reinstated. > Original being "hotplug: Prevent alloc/free..." ? Yes. > > -static int xen_hvm_cpu_notify(struct notifier_block *self, unsigned long action, > - void *hcpu) > +static int xen_cpu_notify(struct notifier_block *self, unsigned long action, > + void *hcpu) > { > int cpu = (long)hcpu; > + int rc; > + > switch (action) { > case CPU_UP_PREPARE: > - xen_vcpu_setup(cpu); > - if (xen_have_vector_callback) { > - if (xen_feature(XENFEAT_hvm_safe_pvclock)) > - xen_setup_timer(cpu); > + if (xen_hvm_domain()) { > + /* > + * This can happen if CPU was offlined earlier and > + * offlining timed out in common_cpu_die(). > + */ > + if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) { > + xen_smp_intr_free(cpu); > + xen_uninit_lock_cpu(cpu); > + } > + > + xen_vcpu_setup(cpu); > } > + > + if (xen_pv_domain() || > + (xen_have_vector_callback && > + xen_feature(XENFEAT_hvm_safe_pvclock))) > + xen_setup_timer(cpu); > + > + rc = xen_smp_intr_init(cpu); > + if (rc) { > + WARN(1, "xen_smp_intr_init() for CPU %d failed: %d\n", > + cpu, rc); > + return NOTIFY_BAD; > + } > + > + break; > + case CPU_ONLINE: > + xen_init_lock_cpu(cpu); > + break; > + case CPU_UP_CANCELED: > + xen_smp_intr_free(cpu); > xen_uninit_lock_cpu ? I don't think this is needed: we initialize lock in CPU_ONLINE notifier which can only be called after CPU_UP_CANCELED would have run (in which case we'll never do CPU_ONLINE) -boris > > >> + if (xen_pv_domain() || >> + (xen_have_vector_callback && >> + xen_feature(XENFEAT_hvm_safe_pvclock))) >> + xen_teardown_timer(cpu); >> break; >> default: >> break;