Re: [Xen-devel] [PATCH v8 11/15] xen: delay allocation of grant table sub structures

[Juergen Gross <jgross@xxxxxxxx>]

On 20/09/17 12:02, Jan Beulich wrote:
>>>> On 20.09.17 at 11:44, <jgross@xxxxxxxx> wrote:
>> On 20/09/17 11:22, Jan Beulich wrote:
>>>>>> On 20.09.17 at 08:34, <jgross@xxxxxxxx> wrote:
>>>> @@ -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);
>>>
>>> These changes don't leave the domains in a state similar to that
>>> before the change - I'm missing calls to gnttab_grow_table() to
>>> establish the minimal sizes. Aiui so far there has been no
>>> requirement for a domain to invoke GNTTABOP_setup_table if it
>>> is happy with v1 and the minimum size.
>>
>> So you don't like gnttab_grow_table() being called when the guest tries
>> to map the grant frames in this case?
> 
> I don't like it being called _only_ in that case.
> 
>> I can add the call of gnttab_grow_table() to grant_table_set_limits() in
>> case you like that better.
> 
> And to grant_table_create() for the Dom0 case.

So in the end: to grant_table_init().


Juergen

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