Re: [Xen-devel] [PATCH v10 5/6] x86/ioreq server: Asynchronously reset outstanding p2m_ioreq_server entries.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.04.17 at 09:18, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
> On 4/3/2017 10:36 PM, Jan Beulich wrote:
>> So this produces the same -EINVAL as the earlier check in context
>> above. I think it would be nice if neither did - -EINUSE for the first
>> (which we don't have, so -EOPNOTSUPP would seem the second
>> bets option there) and -EBUSY for the second would seem more
>> appropriate. If you agree, respective adjustments could be done
>> while committing, if no other reason for a v11 arises.
> 
> Thanks Jan.

First of all - deleting irrelevant context from replies is always
appreciated, but I think you went too far here. It is no longer
possible to re-associate your comments with the change you've
made (not visible from the code fragment below).

> But my code shows both will return -EBUSY, instead of -EINVAL for the 
> mapping requirement:
>        /* Unmap ioreq server from p2m type by passing flags with 0. */
>        if ( flags == 0 )
>        {
> /rc = -EINVAL;/
>            if ( p2m->ioreq.server != s )
>                goto out;
> 
>            p2m->ioreq.server = NULL;
>            p2m->ioreq.flags = 0;
>        }
>        else
>        {
> /rc = -EBUSY;/
>            if ( p2m->ioreq.server != NULL )
>                goto out;
> 
>            /*
>             * It is possible that an ioreq server has just been unmapped,
>             * released the spin lock, with some p2m_ioreq_server entries
>             * in p2m table remained. We shall refuse another ioreq server
>             * mapping request in such case.
>             */
>            if ( read_atomic(&p2m->ioreq.entry_count) )
>                goto out;

Oh, I see: I've mistakenly assumed to addition was to the if()
branch. I clearly didn't look closely enough, I'm sorry.

>            p2m->ioreq.server = s;
>            p2m->ioreq.flags = flags;
>        }
> 
> Are you really wanna use -EOPNOTSUPP when p2m->ioreq.server is not NULL 
> for the mapping code?

As per above, I really did refer to the wrong piece of code, so all
is fine the way the code is.

Jan


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