From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 2/3] Handle GNTST_eagain in kernel drivers Date: Sat, 17 Dec 2011 16:31:25 -0500 Message-ID: <20111217213125.GB17479@phenom.dumpdata.com> References: <1324092141-9730-1-git-send-email-adin@scannell.ca> <1324092141-9730-3-git-send-email-adin@scannell.ca> <20111217143015.GD14816@konrad-lan> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Adin Scannell Cc: olaf@aepfle.de, xen-devel@lists.xensource.com, Adin Scannell , andres@gridcentric.ca, JBeulich@suse.com, Konrad Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org On Sat, Dec 17, 2011 at 11:53:40AM -0500, Adin Scannell wrote: > On Sat, Dec 17, 2011 at 9:30 AM, Konrad Rzeszutek Wilk > wrote: > >> +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p) =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 > > > So why does this have to be a macro? What is the advantage of that > > versus making this a function? > = > I just wanted to minimize changes in the patch from the known XCP > version. I'm personally not a fan of big macros like this. > = > > So why msleep? Why not go for a proper yield? Call the safe_halt() > > function? > = > Yes, this is something that should be revisited. > = > Since it looks like Daniel's HVM patches are going to conflict with > this anyways, I'll revisit this patch independently from the other two > and see what I can do about making it nicer and addressing the other > bits of feedback you've given. OK. Thanks. > = > > Use GNTTABOP_error_msgs. Also should we continue? What is the > > impact if we do continue? The times this is hit is if the status > > is not GNTS_okay and if it is not GNTS_eagain - so what are the > > situations in which this happens and what can the domain do > > to recover? Should there be some helpfull message instead of > > just "gnt status: X" ? > = > In all the cases this macro is called, the caller still needs to check > op.status and handle any errors appropriately. The point of the macro > is to reasonably get you from eagain =3D> !eagain, whatever the result > may be. If I turn this into a function, those semantics will still > apply. OK, it sounds as the 'printk' is not really neccessary as it will be the responsibility of the caller to figure out what to do (which might be very well doing a printk, or maybe not?)