From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chao Peng Subject: Re: [PATCH v3] x86: correct socket_cpumask allocation Date: Mon, 13 Jul 2015 11:19:44 +0800 Message-ID: <20150713031944.GJ3333@pengc-linux.bj.intel.com> References: <1436451837-26171-1-git-send-email-chao.p.peng@linux.intel.com> <1436538580.22672.420.camel@citrix.com> <559FDB1A.8030402@citrix.com> <1436540235.22672.425.camel@citrix.com> <559FFD3F020000780008F9D5@mail.emea.novell.com> <55A00001020000780008F9EF@mail.emea.novell.com> <1436544234.22672.438.camel@citrix.com> Reply-To: Chao Peng Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1436544234.22672.438.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: Andrew Cooper , boris.ostrovsky@oracle.com, keir@xen.org, Jan Beulich , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Fri, Jul 10, 2015 at 06:03:54PM +0200, Dario Faggioli wrote: > On Fri, 2015-07-10 at 16:25 +0100, Jan Beulich wrote: > > >>> On 10.07.15 at 17:13, wrote: > > > > cpu_down() > > > stop_machine_run(take_cpu_down, ...) > > > notifier_call_chain(&cpu_chain, CPU_DYING, ...) > > > __cpu_disable() > > > remove_siblinginfo() > > > __cpu_die() > > > notifier_call_chain(&cpu_chain, CPU_DEAD, ...) > > > cpu_smpboot_free() > > > > > > I.e. a clear use-after-invalidate. > > > > And I can't see a reason why we shouldn't be able to defer invoking > > remove_siblinginfo() until cpu_smpboot_free(): Other than its > > counterpart (set_cpu_sibling_map()) it doesn't require to be run on > > the subject CPU (that function itself doesn't appear to depend on > > that either, but it depends on identify_cpu() having run). Which > > would at once allow reducing code: The clearing of > > cpu_{core,sibling}_mask then becomes redundant with the freeing > > of these masks. > > > > Of course there may be hidden dependencies, so maybe a safer > > approach would be to just move the zapping of the three IDs > > (and maybe the clearing of the CPU's cpu_sibling_setup_map bit) > > into cpu_smpboot_free(). > > cpu_smpboot_free > FWIW, I've tried the patch below (call remove_siblinginfo() from > cpu_smpboot_free()), and it works for me. > > I've tested both shutdown and ACPI S3 suspend. > > I've got to go now, so I guess I'm leaving it to Chao whether to pick it > up, or go with the other approach Jan suggested (or something else). I have no problem if Jan agreed and perhaps especially the OSS test can pass that (as it already looks tricky for this part of code). I can pick this change and send a patch, with the additional change of moving "cpumask_clear_cpu(cpu, socket_cpumask[cpu_to_socket(cpu)])" from remove_siblinginfo() to cpu_smpboot_free(). Chao