From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: -EINTR return in domain_relinquish_resources Date: Fri, 23 Jan 2015 09:32:47 -0500 Message-ID: <20150123143247.GE7365@l.oracle.com> References: <20150121212720.GA24555@l.oracle.com> <54C0D8530200007800057FEB@mail.emea.novell.com> <20150122163840.GB32691@l.oracle.com> <54C21E4D0200007800058992@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YEfI5-00020m-9x for xen-devel@lists.xenproject.org; Fri, 23 Jan 2015 14:32:53 +0000 Content-Disposition: inline In-Reply-To: <54C21E4D0200007800058992@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Fri, Jan 23, 2015 at 09:11:25AM +0000, Jan Beulich wrote: > >>> On 22.01.15 at 17:38, wrote: > > On Thu, Jan 22, 2015 at 10:00:35AM +0000, Jan Beulich wrote: > >> >>> On 21.01.15 at 22:27, 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 > > 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. > > > --- a/xen/arch/x86/mm.c > > +++ b/xen/arch/x86/mm.c > > @@ -2677,6 +2677,9 @@ int vcpu_destroy_pagetables(struct vcpu *v) > > > > v->arch.cr3 = 0; > > > > + if ( rc == -EINTR ) > > + rc = -EAGAIN; > > + > > Either (my preference) you attach this directly to the two > put_page_and_type_preemptible() invocations, or you add a > comment here explaining that this catches those two specific > cases in one central place. This is because while right now only I will add explanation. The other users of put_page_.. all have catch the -EINTR and do the conversion. > those two invocations can lead to rc being non-zero, there's > nothing preventing other error generating code to be added to > this function later on, and we shouldn't blindly convert -EINTR > to some other error code, as that may lead to overlooking > necessary conversions elsewhere (as happened while I made > this function preemptible). > > Jan >