Re: [Xen-devel] [PATCH v2] xen/arm: vcpu: Correctly release resource when the VCPU failed to initialized

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-05-02 at 15:17 +0100, Andrew Cooper wrote:
> 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 <julien.grall@xxxxxxxxxx>
> >> 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.

Ack. #2 (with s/vgic/anything suitably self contained/) is the answer.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel