Re: -EINTR return in domain_relinquish_resources

[Andrew Cooper <andrew.cooper3@citrix.com>]

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@oracle.com> wrote:
>>> On Thu, Jan 22, 2015 at 10:00:35AM +0000, Jan Beulich wrote:
>>>>>>> On 21.01.15 at 22:27, <konrad.wilk@oracle.com> 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@oracle.com>
>>> 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.

However, you patched a more generic path which affects callers, and is
therefore incomplete.  I am sorry if I did not explain this very well.

~Andrew