Re: [Xen-devel] [PATCH v3 3/8] xen: delay allocation of grant table sub structures

[Juergen Gross <jgross@xxxxxxxx>]

On 06/09/17 17:11, Wei Liu wrote:
> On Wed, Sep 06, 2017 at 02:46:48PM +0200, Juergen Gross wrote:
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index 5aebcf265f..11eb1778a3 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -363,6 +363,9 @@ struct domain *domain_create(domid_t domid, unsigned int 
>> domcr_flags,
>>              goto fail;
>>          init_status |= INIT_gnttab;
>>  
>> +        if ( domid == 0 && grant_table_init(d) )
>> +            goto fail;
>> +
>>          poolid = 0;
>>  
>>          err = -ENOMEM;
>> @@ -998,7 +1001,8 @@ int __domain_pause_by_systemcontroller(struct domain *d,
>>          prev = cmpxchg(&d->controller_pause_count, old, new);
>>      } while ( prev != old );
>>  
>> -    pause_fn(d);
>> +    if ( pause_fn )
>> +        pause_fn(d);
>>  
>>      return 0;
>>  }
>> @@ -1006,6 +1010,7 @@ int __domain_pause_by_systemcontroller(struct domain 
>> *d,
>>  int domain_unpause_by_systemcontroller(struct domain *d)
>>  {
>>      int old, new, prev = d->controller_pause_count;
>> +    int ret;
>>  
>>      do
>>      {
>> @@ -1029,8 +1034,16 @@ int domain_unpause_by_systemcontroller(struct domain 
>> *d)
>>       * Creation is considered finished when the controller reference count
>>       * first drops to 0.
>>       */
>> -    if ( new == 0 )
>> +    if ( new == 0 && !d->creation_finished )
>> +    {
> 
> ret can be defined locally here.

Hmm, yes.

> 
>> +        ret = grant_table_init(d);
>> +        if ( ret )
>> +        {
>> +            __domain_pause_by_systemcontroller(d, NULL);
>> +            return ret;
>> +        }
>>          d->creation_finished = true;
>> +    }
>>  
>>      domain_unpause(d);
>>  
>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>> index 4520e36d90..29e7fa539b 100644
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -1655,6 +1655,78 @@ gnttab_unpopulate_status_frames(struct domain *d, 
>> struct grant_table *gt)
>>      gt->nr_status_frames = 0;
>>  }
>>  
>> +int
>> +grant_table_init(struct domain *d)
>> +{
>> +    struct grant_table *gt = d->grant_table;
>> +    unsigned int i, j;
>> +
>> +    if ( gt->nr_grant_frames )
>> +        return 0;
>> +
> 
> EBUSY here? I think we should catch the cases when this is called
> multiple times.

No. The call of grant_table_init() from
domain_unpause_by_systemcontroller() can't be masked, otherwise I
would have to make struct grant_table public again. Multiple calls
are okay.

> 
> With those changed:
> 
> Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> 

Juergen

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