From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2] xen/arm: vcpu: Correctly release resource when the VCPU failed to initialized Date: Fri, 2 May 2014 15:17:20 +0100 Message-ID: <5363A8F0.2050109@citrix.com> References: <1398885355-13200-1-git-send-email-julien.grall@linaro.org> <1399033506.32736.84.camel@kazak.uk.xensource.com> <5363A735.3020205@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WgEHi-0006E4-F4 for xen-devel@lists.xenproject.org; Fri, 02 May 2014 14:17:54 +0000 In-Reply-To: <5363A735.3020205@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: xen-devel@lists.xenproject.org, tim@xen.org, Ian Campbell , stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org On 02/05/14 15:09, Julien Grall wrote: > On 05/02/2014 01:25 PM, Ian Campbell wrote: >> On Wed, 2014-04-30 at 20:15 +0100, Julien Grall wrote: >>> While I was adding new failing code at the end of the function, I've noticed >>> that the vtimers are not freed which mess all the timers and will crash Xen >>> quickly when the page will be reused. >>> >>> Currently neither vcpu_vgic_init nor vcpu_vtimer_init fail, so we >>> are safe for now. With the new GICv3 code, the former function will be able >>> to fail. This will result to a memory leak. >>> >>> Call vcpu_destroy if the initialization has failed. We also need to add a >>> boolean to know if the vtimers are correctly setup as the timer common code >>> doesn't have safe guard against removing non-initialized timer. >>> >>> Signed-off-by: Julien Grall >> I was about to acked + apply but it failed to build on arm64 with: >> >> domain.c: In function 'alloc_vcpu_struct': >> /local/scratch/ianc/devel/committer.git/xen/include/xen/lib.h:19:31: error: static assertion failed: "!(sizeof(*v) > PAGE_SIZE)" >> #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); }) >> ^ >> domain.c:415:5: note: in expansion of macro 'BUILD_BUG_ON' >> BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE); >> ^ >> struct arch_vcpu is apparently now too large. > Hmmm... I'm not sure what is the best solution. Can: > 1) Allocate 2 pages for the VCPU structure > 2) Allocate vgic structure outside. > > Any opinions? > > Regards, > 2) The reason structs vcpu/domain were reduced to this size to was avoid needing multi-page allocations, which risk allocation failures on systems with sufficiently fragmented memory. ~Andrew