Re: [Xen-devel] Grant unmap error checking in Dom0

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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

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