Re: [BUG] Xen vm kernel crash in get_free_entries.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Nov 27, 2013 at 03:58:55PM +0000, Ian Campbell wrote:
> On Wed, 2013-11-27 at 09:24 -0500, Konrad Rzeszutek Wilk wrote:
> > On Wed, Nov 27, 2013 at 09:36:55AM +0000, Ian Campbell wrote:
> > > On Tue, 2013-11-26 at 15:08 -0500, Konrad Rzeszutek Wilk wrote:
> > > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> > > > index 432db1b..bcbaf0b 100644
> > > > --- a/drivers/block/xen-blkfront.c
> > > > +++ b/drivers/block/xen-blkfront.c
> > > > @@ -2074,7 +2074,7 @@ static int __init xlblk_init(void)
> > > >  	if (!xen_domain())
> > > >  		return -ENODEV;
> > > >  
> > > > -	if (xen_hvm_domain() && !xen_platform_pci_unplug)
> > > > +	if (xen_err_out())
> > > 
> > > I think !xen_has_pv_devices() or some such would be a better name.
> > 
> > <nods>
> > > > diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h
> > > > index 438c256..a5bbd0b 100644
> > > > --- a/include/xen/platform_pci.h
> > > > +++ b/include/xen/platform_pci.h
> > > > @@ -47,5 +47,18 @@ static inline int xen_must_unplug_disks(void) {
> > > >  }
> > > >  
> > > >  extern int xen_platform_pci_unplug;
> > > > +static  bool xen_err_out(void)
> > > 
> > >          ^ stray space, but I think you wanted an inline here anyway?
> > 
> > Yup.
> > > 
> > > Or you could move this to arch/x86/xen/platform-pci-unplug.c and then
> > > xen_platform_pci_unplug could be unexported, which seems like a good
> > > thing to do if the logic to using it is as complex as below.
> > 
> > I was thinking about it - but then there is a bit of a problem with
> > !CONFIG_PVHVM && CONFIG_XEN_BLKFRONT for example. Which means that
> > platform-pci-unplug.c won't be built, but the xen-blkfront will and
> > it needs the xen_has_pv_devices()). Hence sticking it in a header.
> 
> Is that not just a case of #define xen_has_pc_devices 1 with the
> appropriate ifndef CONFIG_PVHVM in the header (the other case being the
> prototype for the out of line version)? That's a pretty common pattern
> for things which rely on a patricular CONFIG_FOO to be useful.

Yes. That would do it too (duh!)
> 
> Ian.
>