From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chao Peng Subject: Re: [PATCH v2] x86: correct socket_cpumask allocation Date: Thu, 9 Jul 2015 22:36:14 +0800 Message-ID: <20150709143614.GI3333@pengc-linux.bj.intel.com> References: <1436430376-31276-1-git-send-email-chao.p.peng@linux.intel.com> <559E5E03020000780008EA88@mail.emea.novell.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: <559E5E03020000780008EA88@mail.emea.novell.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: Jan Beulich Cc: andrew.cooper3@citrix.com, dario.faggioli@citrix.com, keir@xen.org, boris.ostrovsky@oracle.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Thu, Jul 09, 2015 at 10:41:55AM +0100, Jan Beulich wrote: > >>> On 09.07.15 at 10:26, wrote: > > @@ -748,8 +758,9 @@ static int cpu_smpboot_alloc(unsigned int cpu) > > goto oom; > > per_cpu(stubs.addr, cpu) = stub_page + STUB_BUF_CPU_OFFS(cpu); > > > > - if ( !socket_cpumask[socket] && > > - !zalloc_cpumask_var(socket_cpumask + socket) ) > > + if ( secondary_socket_cpumask == NULL && > > + (secondary_socket_cpumask = _xzalloc(nr_cpumask_bits / 8, > > + sizeof(long))) == NULL ) > > This is horrible since completely type-unsafe, and correct only > because _xmalloc() happens to allocate more space than requested > if the size isn't a multiple of MEM_ALIGN. And it makes me realize why > on IRC I first suggested xzalloc_array(): That would at least have > taken care of that latent bug. And remember that I did _not_ > suggest _xzalloc(), but xzalloc(). > > Taken together I think we should stay with using zalloc_cpumask_var(), > and introduce zap_cpumask_var() (storing NULL in the big NR_CPUS > case and doing nothing in the small one). Apart from zap_cpumask_var() there is need to check if cpumask_vat_t is NULL as well. While that is weird to satisfy compiler for small NR_CPUS case. > Should I be overlooking > something that still prevents this from building in both cases, the > above allocation should be changed to at least be type safe (and I > guess I'd rather waste a few bytes here than see you add fragile > casts or some such). So this solution is finally adopted. The new version is already sent out. Chao