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 16:15 +0100, Ian Campbell wrote:
> 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.

Was Vijay not moving the vgtic stuff out in one of the gicv3 patches?



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