From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [BUG] Xen vm kernel crash in get_free_entries. Date: Wed, 27 Nov 2013 09:24:46 -0500 Message-ID: <20131127142446.GD4430@pegasus.dumpdata.com> References: <5262642A.2060609@rat.ru> <1382180597.28188.9.camel@dagon.hellion.org.uk> <267039552.20131019135850@eikelenboom.it> <20131021105510.GB12019@u109add4315675089e695.ant.amazon.com> <527B2311.2030605@rat.ru> <1383832023.32399.39.camel@kazak.uk.xensource.com> <20131112155614.GA11354@phenom.dumpdata.com> <1384335658.5406.16.camel@kazak.uk.xensource.com> <20131126200847.GA12670@phenom.dumpdata.com> <1385545015.23112.122.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1385545015.23112.122.camel@kazak.uk.xensource.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: Ian Campbell Cc: Steven Noonan , Stefano Stabellini , Matt Wilson , xen-devel@lists.xen.org, Sander Eikelenboom , astarta@rat.ru, David Vrabel , Matt Wilson List-Id: xen-devel@lists.xenproject.org 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. > > 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. > > > +{ > > > > + if (!xen_domain()) > > + return true; > > + > > + if (xen_hvm_domain()) { > > + if (xen_platform_pci_unplug & (XEN_UNPLUG_UNNECESSARY | XEN_UNPLUG_NEVER)) > > + return true; > > + if (xen_platform_pci_unplug == 0) > > + return true; > > There are some cases where xen_unplug_emulated_devices doesn't update > xen_platform_pci_unplug, specifically when xen_emul_unplug contains > UNNECESSARY or NEVER. I think the above logic covers that case but it > might be simpler to always set it? At that point on the first if is > necessary? Thanks for your review! > > > + } > > + return false; > > +} > > #endif /* _XEN_PLATFORM_PCI_H */ > >