Re: [Xen-devel] [PATCH v5 4/4] x86/ioreq server: Reset outstanding p2m_ioreq_server entries when an ioreq server unmaps.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.08.16 at 09:39, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:

> 
> On 8/9/2016 12:29 AM, Jan Beulich wrote:
>>>>> On 12.07.16 at 11:02, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
>>> @@ -5512,6 +5513,12 @@ static int hvmop_set_mem_type(
>>>           if ( rc )
>>>               goto out;
>>>   
>>> +        if ( t == p2m_ram_rw && memtype[a.hvmmem_type] == p2m_ioreq_server 
>>> )
>>> +            p2m->ioreq.entry_count++;
>>> +
>>> +        if ( t == p2m_ioreq_server && memtype[a.hvmmem_type] == p2m_ram_rw 
>>> )
>>> +            p2m->ioreq.entry_count--;
>>> +
>> These (and others below) happen, afaict, outside of any lock, which
>> can't be right.
> 
> How about we make this entry_count as atomic_t and use atomic_inc/dec 
> instead?

That's certainly one of the options, but beware of overflow.

>>> +    /*
>>> +     * Iterate p2m table when an ioreq server unmaps from p2m_ioreq_server,
>>> +     * and reset the remaining p2m_ioreq_server entries back to p2m_ram_rw.
>>> +     */
>>> +    if ( op.flags == 0 && rc == 0 )
>>> +    {
>>> +        struct p2m_domain *p2m = p2m_get_hostp2m(d);
>>> +
>>> +        while ( gfn <= p2m->max_mapped_pfn )
>>> +        {
>>> +            p2m_type_t t;
>>> +
>>> +            if ( p2m->ioreq.entry_count == 0 )
>>> +                break;
>> Perhaps better to be moved up into the while() expression?
> 
> OK. Another thing is maybe we can use _atomic_read() to get value of 
> entry_count if
> the counter is defined as atomic_t?

Well, that's an orthogonal adjustment which solely depends on the
type of the field.

Jan


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