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

[Juergen Gross <jgross@xxxxxxxx>]

On 19/09/17 12:28, Paul Durrant wrote:
>> -----Original Message-----
>> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of
>> Juergen Gross
>> Sent: 19 September 2017 10:59
>> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Cc: Juergen Gross <jgross@xxxxxxxx>; sstabellini@xxxxxxxxxx; Wei Liu
>> <wei.liu2@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>;
>> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson
>> <Ian.Jackson@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>;
>> julien.grall@xxxxxxx; jbeulich@xxxxxxxx; dgdegra@xxxxxxxxxxxxx
>> 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().


Juergen

> 
>   Paul
> 
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxx
>> https://lists.xen.org/xen-devel


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