Re: [PATCH v7 12/16] xen: delay allocation of grant table sub structures

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Juergen Gross [mailto:jgross@suse.com]
> Sent: 19 September 2017 11:36
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: sstabellini@kernel.org; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; julien.grall@arm.com; jbeulich@suse.com;
> dgdegra@tycho.nsa.gov
> Subject: Re: [Xen-devel] [PATCH v7 12/16] xen: delay allocation of grant table
> sub structures
> 
> On 19/09/17 12:28, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of
> >> Juergen Gross
> >> Sent: 19 September 2017 10:59
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Juergen Gross <jgross@suse.com>; sstabellini@kernel.org; Wei Liu
> >> <wei.liu2@citrix.com>; George Dunlap <George.Dunlap@citrix.com>;
> >> Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> >> <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>;
> >> julien.grall@arm.com; jbeulich@suse.com; dgdegra@tycho.nsa.gov
> >> Subject: [Xen-devel] [PATCH v7 12/16] xen: delay allocation of grant table
> >> sub structures
> >>
> >> Delay the allocation of the grant table sub structures in order to
> >> allow modifying parameters needed for sizing of these structures at a
> >> per domain basis. Allocate the structures from gnttab_setup_table()
> >> and the table frames only from gnttab_grow_table().
> >
> > Is this last sentence still correct?
> 
> Only partially.
> 
> The structures are now allocated from grant_table_set_limits() and for
> dom0 from grant_table_create().
> 

That was my reading too... Just wasn't sure I was being fooled by the large number of diffs. With the comment fixed:

Reveiwed-by: Paul Durrant <paul.durrant@citrix.com>

> 
> Juergen
> 
> >
> >   Paul
> >
> >>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> >> ---
> >> V6:
> >> - move call of grant_table_init() for dom0 to grant_table_create()
> >>   (Jan Beulich)
> >> - move frame allocations to gnttab_grow_table() (Jan Beulich)
> >> - several other changes due to new patch order
> >>
> >> V4:
> >> - make ret more local (Wei Liu)
> >>
> >> V3:
> >> - move call of grant_table_init() from gnttab_setup_table() to
> >>   gnttab_grow_table() (Paul Durrant)
> >> ---
> >>  xen/common/grant_table.c | 113 ++++++++++++++++++++++-------------
> ---
> >> ---------
> >>  1 file changed, 52 insertions(+), 61 deletions(-)
> >>
> >> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> >> index f48eeff7ad..f66940551e 100644
> >> --- a/xen/common/grant_table.c
> >> +++ b/xen/common/grant_table.c
> >> @@ -1655,6 +1655,46 @@ gnttab_unpopulate_status_frames(struct
> domain
> >> *d, struct grant_table *gt)
> >>      gt->nr_status_frames = 0;
> >>  }
> >>
> >> +static int
> >> +grant_table_init(struct grant_table *gt)
> >> +{
> >> +    if ( gt->active )
> >> +        return -EBUSY;
> >> +
> >> +    /* Active grant table. */
> >> +    gt->active = xzalloc_array(struct active_grant_entry *,
> >> +                               max_nr_active_grant_frames);
> >> +    if ( gt->active == NULL )
> >> +        goto no_mem;
> >> +
> >> +    /* Tracking of mapped foreign frames table */
> >> +    gt->maptrack = vzalloc(max_maptrack_frames * sizeof(*gt-
> >maptrack));
> >> +    if ( gt->maptrack == NULL )
> >> +        goto no_mem;
> >> +
> >> +    /* Shared grant table. */
> >> +    gt->shared_raw = xzalloc_array(void *, max_grant_frames);
> >> +    if ( gt->shared_raw == NULL )
> >> +        goto no_mem;
> >> +
> >> +    /* Status pages for grant table - for version 2 */
> >> +    gt->status = xzalloc_array(grant_status_t *,
> >> +                               grant_to_status_frames(max_grant_frames));
> >> +    if ( gt->status == NULL )
> >> +        goto no_mem;
> >> +
> >> +    return 0;
> >> +
> >> + no_mem:
> >> +    xfree(gt->shared_raw);
> >> +    gt->shared_raw = NULL;
> >> +    vfree(gt->maptrack);
> >> +    gt->maptrack = NULL;
> >> +    xfree(gt->active);
> >> +    gt->active = NULL;
> >> +    return -ENOMEM;
> >> +}
> >> +
> >>  /*
> >>   * Grow the grant table. The caller must hold the grant table's
> >>   * write lock before calling this function.
> >> @@ -1665,6 +1705,10 @@ gnttab_grow_table(struct domain *d, unsigned
> int
> >> req_nr_frames)
> >>      struct grant_table *gt = d->grant_table;
> >>      unsigned int i, j;
> >>
> >> +    ASSERT(gt->active);
> >> +
> >> +    if ( req_nr_frames < INITIAL_NR_GRANT_FRAMES )
> >> +        req_nr_frames = INITIAL_NR_GRANT_FRAMES;
> >>      ASSERT(req_nr_frames <= max_grant_frames);
> >>
> >>      gdprintk(XENLOG_INFO,
> >> @@ -3381,75 +3425,21 @@ grant_table_create(
> >>      struct domain *d)
> >>  {
> >>      struct grant_table *t;
> >> -    unsigned int i, j;
> >>
> >>      if ( (t = xzalloc(struct grant_table)) == NULL )
> >> -        goto no_mem_0;
> >> +        return -ENOMEM;
> >>
> >>      /* Simple stuff. */
> >>      percpu_rwlock_resource_init(&t->lock, grant_rwlock);
> >>      spin_lock_init(&t->maptrack_lock);
> >> -    t->nr_grant_frames = INITIAL_NR_GRANT_FRAMES;
> >> -
> >> -    /* Active grant table. */
> >> -    if ( (t->active = xzalloc_array(struct active_grant_entry *,
> >> -                                    max_nr_active_grant_frames)) == NULL )
> >> -        goto no_mem_1;
> >> -    for ( i = 0;
> >> -          i <
> num_act_frames_from_sha_frames(INITIAL_NR_GRANT_FRAMES);
> >> i++ )
> >> -    {
> >> -        if ( (t->active[i] = alloc_xenheap_page()) == NULL )
> >> -            goto no_mem_2;
> >> -        clear_page(t->active[i]);
> >> -        for ( j = 0; j < ACGNT_PER_PAGE; j++ )
> >> -            spin_lock_init(&t->active[i][j].lock);
> >> -    }
> >> -
> >> -    /* Tracking of mapped foreign frames table */
> >> -    t->maptrack = vzalloc(max_maptrack_frames * sizeof(*t->maptrack));
> >> -    if ( t->maptrack == NULL )
> >> -        goto no_mem_2;
> >> -
> >> -    /* Shared grant table. */
> >> -    if ( (t->shared_raw = xzalloc_array(void *, max_grant_frames)) ==
> NULL )
> >> -        goto no_mem_3;
> >> -    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
> >> -    {
> >> -        if ( (t->shared_raw[i] = alloc_xenheap_page()) == NULL )
> >> -            goto no_mem_4;
> >> -        clear_page(t->shared_raw[i]);
> >> -    }
> >> -
> >> -    /* Status pages for grant table - for version 2 */
> >> -    t->status = xzalloc_array(grant_status_t *,
> >> -                              grant_to_status_frames(max_grant_frames));
> >> -    if ( t->status == NULL )
> >> -        goto no_mem_4;
> >> -
> >> -    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
> >> -        gnttab_create_shared_page(d, t, i);
> >> -
> >> -    t->nr_status_frames = 0;
> >>
> >>      /* Okay, install the structure. */
> >>      d->grant_table = t;
> >> -    return 0;
> >>
> >> - no_mem_4:
> >> -    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
> >> -        free_xenheap_page(t->shared_raw[i]);
> >> -    xfree(t->shared_raw);
> >> - no_mem_3:
> >> -    vfree(t->maptrack);
> >> - no_mem_2:
> >> -    for ( i = 0;
> >> -          i <
> num_act_frames_from_sha_frames(INITIAL_NR_GRANT_FRAMES);
> >> i++ )
> >> -        free_xenheap_page(t->active[i]);
> >> -    xfree(t->active);
> >> - no_mem_1:
> >> -    xfree(t);
> >> - no_mem_0:
> >> -    return -ENOMEM;
> >> +    if ( d->domain_id == 0 )
> >> +        return grant_table_init(t);
> >> +
> >> +    return 0;
> >>  }
> >>
> >>  void
> >> @@ -3651,8 +3641,9 @@ int grant_table_set_limits(struct domain *d,
> >> unsigned int grant_frames,
> >>
> >>      grant_write_lock(gt);
> >>
> >> -    ret = 0;
> >> -    /* Set limits, alloc needed arrays. */
> >> +    /* Set limits. */
> >> +    if ( !gt->active )
> >> +        ret = grant_table_init(gt);
> >>
> >>      grant_write_unlock(gt);
> >>
> >> --
> >> 2.12.3
> >>
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xen.org
> >> https://lists.xen.org/xen-devel

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