[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/2] domain: use PGC_extra domheap page for shared_info



On 26.02.2020 12:33, Julien Grall wrote:
> On 25/02/2020 09:53, Paul Durrant wrote:
>> --- a/xen/common/time.c
>> +++ b/xen/common/time.c
>> @@ -99,6 +99,9 @@ void update_domain_wallclock_time(struct domain *d)
>>       uint32_t *wc_version;
>>       uint64_t sec;
>>   
>> +    if ( d->is_dying )
>> +        return;
> 
> This is another instance where I think the use of d->is_dying is not 
> safe. I looked at how other places in Xen dealt with d->is_dying.
> 
> We don't seem to have a common way to deal with it:
>     1) It may be checked under domain_lock() -> No issue with that
>     2) It may be checked under d->page_alloc_lock (e.g assign_pages()). 
> The assign_pages() case is fine because it will act as a full barrier. 
> So if we call happen after relinquish_memory() then we will surely have 
> observed d->is_dying. I haven't checked the others.
> 
> Some of the instances user neither the 2 locks above. We probably ought 
> to investigate them (I will add a TODO in my list).
> 
> Regarding the two cases here, domain_lock() might be the solution. If we 
> are concern about the contention (it is a spinlock), then we could 
> switch the domain_lock() from spinlock to rwlock.

That's an option, yes, but even better would be to avoid spreading
use of this lock (we did try to remove as many of the uses as
possible several years ago). For d->is_dying I think it is a case-
by-case justification of why a use is safe (the main point, after
all being that whatever code comes after the check must work
correctly if on some other CPU the flag is about to be / being set.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.