From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: Grant unmap error checking in Dom0 Date: Wed, 19 Mar 2014 08:56:26 -0400 Message-ID: <20140319125626.GA8694@phenom.dumpdata.com> References: <5328484E.5060701@citrix.com> <20140318133834.GC25364@phenom.dumpdata.com> <53286980.3050204@citrix.com> <20140318155403.GB28647@phenom.dumpdata.com> <5328C120.5060400@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <5328C120.5060400@citrix.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: Zoltan Kiss Cc: Ian Campbell , David Vrabel , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Tue, Mar 18, 2014 at 09:56:48PM +0000, Zoltan Kiss wrote: > On 18/03/14 15:54, Konrad Rzeszutek Wilk wrote: > >On Tue, Mar 18, 2014 at 03:42:56PM +0000, Zoltan Kiss wrote: > >>On 18/03/14 13:38, Konrad Rzeszutek Wilk wrote: > >>>On Tue, Mar 18, 2014 at 01:21:18PM +0000, Zoltan Kiss wrote: > >>>>Hi, > >>>> > >>>>Just out of curiosity I've checked how Dom0 handles errors during > >>>>grant unmapping. Usually there is a BUG_ON(ret) for the return value > >>>>of gnttab_unmap_refs in blkback and netback, gntdev drops just a > >>>>WARN. > >>>>The return value can be non-zero only if Xen failed to copy the map > >>>>operations back and forth to the guest supplied memory, so it's > >>>>reasonable to crash there. However I'm wondering why gntdev is happy > >>>>with just a WARN. > >>>>Another thing, we don't check the status of the operations if the > >>>>return value is zero. We shouldn't normally do that, Xen logs info > >>>>messages in some cases, but not always (e.g. XSM or IOMMU problems). > >>>>For debugging purposes however it could be useful to have the > >>>>ability to turn on checking in Dom0. A quick and dirty way to do > >>>>this is to use printk_get_level to figure out if the loglevel is > >>>>e.g. KERN_NOTICE or lower, but I'm sure there is a better way to do > >>>>this :) It would be an overkill to introduce new config option, I'm > >>>>thinking a runtime parameter to check in an unlikely(), so it won't > >>>>cause performance penalty for normal operation. Any opinions on > >>>>that? > >>> > >>>One can always just have printk(KERN_DEBUG and if the user did not > >>>boot with 'debug' the messages go to /dev/null. > >>Checking through the unmap_ops array itself can take a considerable > >>amount of cycles, I think it would be better to avoid that. > > > > > >I was thinking it was triggered on your 'return value'. > > > >In this case I would say just use asm goto (jump labels) and maybe have > >it turned on via debugfs? That would make all of this based on the > >CONFIG_XEN_DEBUGFS? > I would prefer avoiding the introduction of a new config parameter, > if possible. A module parameter rather, but as I'm lazy, and the > importance of this checking is not too high, the best would be to > bind this to some general "is debugging turned on?" option. > If there would be a shorthand function "is_debugfs_mounted", that > would be reasonable, but I couldn't find it. In that case I would just say go for 'early_bootparam' or a normal parameter and make it use the jump label machinery. See arch/x86/xen/spinlock.c for the xen_pvspin parameter as an example. > > Zoli