Re: [Xen-devel] -EINTR return in domain_relinquish_resources

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.01.15 at 15:46, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 23/01/15 14:32, Konrad Rzeszutek Wilk wrote:
>> On Fri, Jan 23, 2015 at 09:11:25AM +0000, Jan Beulich wrote:
>>>>>> On 22.01.15 at 17:38, <konrad.wilk@xxxxxxxxxx> wrote:
>>>> On Thu, Jan 22, 2015 at 10:00:35AM +0000, Jan Beulich wrote:
>>>>>>>> On 21.01.15 at 22:27, <konrad.wilk@xxxxxxxxxx> wrote:
>>>>>> As I was looking at some of the XSA I realized that the
>>>>>> call-chain of:
>>>>>>
>>>>>>  domain_relinquish_resources
>>>>>>    ->vcpu_destroy_pagetables
>>>>>>      -> put_page_and_type_preemptible
>>>>>>         -> __put_page_type
>>>>>>          returns -EINTR
>>>>>> [...]
>>>>>>  b). Or should the hypervisor convert the -EINTR to -ERESTART
>>>>>>      (or -EAGAIN) - which most of the code (see users of
>>>>>>      get_page_type_preemptible) do right now?
>>>>> This one, in vcpu_destroy_pagetables(). I'm afraid I overlooked
>>>>> this when adding the preemption capability here.
>>>> OK. Would this RFC patch be OK? (I can send it off as normal - just
>>>> want to make sure you are OK with this being put there)
>>> Conceptually yes, but there are issues:
>>>
>>>> From 1558a9870f438df949a8ad09a27825bd35a9f4ea Mon Sep 17 00:00:00 2001
>>>> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>>>> Date: Thu, 22 Jan 2015 11:34:21 -0500
>>>> Subject: [PATCH] domain: In vcpu_destroy_pagetables we can return -EINTR
>>>>  instead of -EAGAIN
>>> You should stop sending such conversions to EAGAIN. We switched
>>> to ERESTART, and you (just guessing) taking the patch from an
>>> older Xen version shouldn't result in this recurring mistake.
>> Nah, Andrew said in his email EAGAIN so that is what I picked.
> 
> EAGAIN was correct for the domain_destroy hypercall, at this hypercall
> explicitly has continuation built into its API via EAGAIN.

Ouch - I based the comment on code resulting from a patch I
didn't send out yet (largely because Konrad indicated issues with
XEN_DOMCTL_destroydomain that I'd need to look into in more
detail before doing so), doing away with the tool stack based
continuations. Yet based on what domain_kill() does with
domain_relinquish_resources()'s return value, it should
nevertheless be -ERESTART here to ease future changes.

Jan


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