Frank Pan
2011-Mar-03 16:46 UTC
[Xen-devel] [PATCH pvops] Fix a bug that shutdown watcher is disabled on pvops
I use pvops kernel for HVM domU, and I find that shutdown watcher is not enabled at all when I compile platform-pci module inside kernel. The code related calls xen_setup_shutdown_event after xenbus_probe, that makes setup_shutdown_watcher never called. The following patch fixes this by changing the order of xenbus_probe and xen_setup_shutdown, which makes my PV-on-HVM domU responses well for the "control/shutdown" entry. --- linux-2.6-xen/drivers/xen/platform-pci.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/linux-2.6-xen/drivers/xen/platform-pci.c b/linux-2.6-xen/drivers/xen/platform-pci.c index c01b5dd..f141cfe 100644 --- a/linux-2.6-xen/drivers/xen/platform-pci.c +++ b/linux-2.6-xen/drivers/xen/platform-pci.c @@ -162,10 +162,10 @@ static int __devinit platform_pci_init(struct pci_dev *pdev, ret = gnttab_init(); if (ret) goto out; - xenbus_probe(NULL); ret = xen_setup_shutdown_event(); if (ret) goto out; + xenbus_probe(NULL); return 0; out: -- 1.7.0.4 -- Frank Pan Computer Science and Technology Tsinghua University _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Mar-03 18:29 UTC
Re: [Xen-devel] [PATCH pvops] Fix a bug that shutdown watcher is disabled on pvops
On Fri, Mar 04, 2011 at 12:46:20AM +0800, Frank Pan wrote:> I use pvops kernel for HVM domU, and I find that shutdown watcher isWhich one? 2.6.32? stock 2.6.37? devel 2.6.38?> not enabled at all when I compile platform-pci module inside kernel. > The code related calls xen_setup_shutdown_event after xenbus_probe, > that makes setup_shutdown_watcher never called. > > The following patch fixes this by changing the order of xenbus_probe > and xen_setup_shutdown, which makes my PV-on-HVM domU responses well > for the "control/shutdown" entry. > > --- > linux-2.6-xen/drivers/xen/platform-pci.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/linux-2.6-xen/drivers/xen/platform-pci.c > b/linux-2.6-xen/drivers/xen/platform-pci.c > index c01b5dd..f141cfe 100644 > --- a/linux-2.6-xen/drivers/xen/platform-pci.c > +++ b/linux-2.6-xen/drivers/xen/platform-pci.c > @@ -162,10 +162,10 @@ static int __devinit platform_pci_init(struct > pci_dev *pdev, > ret = gnttab_init(); > if (ret) > goto out; > - xenbus_probe(NULL); > ret = xen_setup_shutdown_event(); > if (ret) > goto out; > + xenbus_probe(NULL); > return 0; > > out: > -- > 1.7.0.4 > > -- > Frank Pan > > Computer Science and Technology > Tsinghua University> From 8d2402bd48ccc9eb5afe2aaa407d61ff715cb24c Mon Sep 17 00:00:00 2001 > From: Frank Pan <frankpzh@gmail.com> > Date: Thu, 3 Mar 2011 16:36:17 +0000 > Subject: [PATCH] Fix a bug that shutdown watcher is disabled on pvops > > --- > linux-2.6-xen/drivers/xen/platform-pci.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/linux-2.6-xen/drivers/xen/platform-pci.c b/linux-2.6-xen/drivers/xen/platform-pci.c > index c01b5dd..f141cfe 100644 > --- a/linux-2.6-xen/drivers/xen/platform-pci.c > +++ b/linux-2.6-xen/drivers/xen/platform-pci.c > @@ -162,10 +162,10 @@ static int __devinit platform_pci_init(struct pci_dev *pdev, > ret = gnttab_init(); > if (ret) > goto out; > - xenbus_probe(NULL); > ret = xen_setup_shutdown_event(); > if (ret) > goto out; > + xenbus_probe(NULL); > return 0; > > out: > -- > 1.7.0.4 >> _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Frank Pan
2011-Mar-04 02:09 UTC
Re: [Xen-devel] [PATCH pvops] Fix a bug that shutdown watcher is disabled on pvops
Sorry I didn''t mention it. I use jeremy''s tree. git clone git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git linux-2.6-xen The kernel version is 2.6.32.27. On Fri, Mar 4, 2011 at 2:29 AM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:> On Fri, Mar 04, 2011 at 12:46:20AM +0800, Frank Pan wrote: >> I use pvops kernel for HVM domU, and I find that shutdown watcher is > > Which one? 2.6.32? stock 2.6.37? devel 2.6.38? >> not enabled at all when I compile platform-pci module inside kernel. >> The code related calls xen_setup_shutdown_event after xenbus_probe, >> that makes setup_shutdown_watcher never called. >> >> The following patch fixes this by changing the order of xenbus_probe >> and xen_setup_shutdown, which makes my PV-on-HVM domU responses well >> for the "control/shutdown" entry. >> >> --- >> linux-2.6-xen/drivers/xen/platform-pci.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/linux-2.6-xen/drivers/xen/platform-pci.c >> b/linux-2.6-xen/drivers/xen/platform-pci.c >> index c01b5dd..f141cfe 100644 >> --- a/linux-2.6-xen/drivers/xen/platform-pci.c >> +++ b/linux-2.6-xen/drivers/xen/platform-pci.c >> @@ -162,10 +162,10 @@ static int __devinit platform_pci_init(struct >> pci_dev *pdev, >> ret = gnttab_init(); >> if (ret) >> goto out; >> - xenbus_probe(NULL); >> ret = xen_setup_shutdown_event(); >> if (ret) >> goto out; >> + xenbus_probe(NULL); >> return 0; >> >> out: >> -- >> 1.7.0.4 >> >> -- >> Frank Pan >> >> Computer Science and Technology >> Tsinghua University > >> From 8d2402bd48ccc9eb5afe2aaa407d61ff715cb24c Mon Sep 17 00:00:00 2001 >> From: Frank Pan <frankpzh@gmail.com> >> Date: Thu, 3 Mar 2011 16:36:17 +0000 >> Subject: [PATCH] Fix a bug that shutdown watcher is disabled on pvops >> >> --- >> linux-2.6-xen/drivers/xen/platform-pci.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/linux-2.6-xen/drivers/xen/platform-pci.c b/linux-2.6-xen/drivers/xen/platform-pci.c >> index c01b5dd..f141cfe 100644 >> --- a/linux-2.6-xen/drivers/xen/platform-pci.c >> +++ b/linux-2.6-xen/drivers/xen/platform-pci.c >> @@ -162,10 +162,10 @@ static int __devinit platform_pci_init(struct pci_dev *pdev, >> ret = gnttab_init(); >> if (ret) >> goto out; >> - xenbus_probe(NULL); >> ret = xen_setup_shutdown_event(); >> if (ret) >> goto out; >> + xenbus_probe(NULL); >> return 0; >> >> out: >> -- >> 1.7.0.4 >> > >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > >-- 潘震皓, Frank Pan Computer Science and Technology Tsinghua University _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Frank Pan
2011-Mar-04 02:39 UTC
Re: [Xen-devel] [PATCH pvops] Fix a bug that shutdown watcher is disabled on pvops
> Which one? 2.6.32? stock 2.6.37? devel 2.6.38?I used branch xen/next-2.6.32 for testing. A quick code check shows this issue should also exist on: xen/next-2.6.37 xen/next-2.6.38 xen/stable xen/stable-2.6.32.x But I need tests on them. -- Frank Pan Computer Science and Technology Tsinghua University _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Frank Pan
2011-Mar-04 02:57 UTC
Re: [Xen-devel] [PATCH pvops] Fix a bug that shutdown watcher is disabled on pvops
Branch devel/next-2.6.38 on your tree has no such issue. In this branch, setup_shutdown_watcher will be called inside xen_setup_shutdown_event.(on PV-on-HVM) On Fri, Mar 4, 2011 at 2:29 AM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:> On Fri, Mar 04, 2011 at 12:46:20AM +0800, Frank Pan wrote: >> I use pvops kernel for HVM domU, and I find that shutdown watcher is > > Which one? 2.6.32? stock 2.6.37? devel 2.6.38? >> not enabled at all when I compile platform-pci module inside kernel. >> The code related calls xen_setup_shutdown_event after xenbus_probe, >> that makes setup_shutdown_watcher never called. >> >> The following patch fixes this by changing the order of xenbus_probe >> and xen_setup_shutdown, which makes my PV-on-HVM domU responses well >> for the "control/shutdown" entry. >> >> --- >> linux-2.6-xen/drivers/xen/platform-pci.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/linux-2.6-xen/drivers/xen/platform-pci.c >> b/linux-2.6-xen/drivers/xen/platform-pci.c >> index c01b5dd..f141cfe 100644 >> --- a/linux-2.6-xen/drivers/xen/platform-pci.c >> +++ b/linux-2.6-xen/drivers/xen/platform-pci.c >> @@ -162,10 +162,10 @@ static int __devinit platform_pci_init(struct >> pci_dev *pdev, >> ret = gnttab_init(); >> if (ret) >> goto out; >> - xenbus_probe(NULL); >> ret = xen_setup_shutdown_event(); >> if (ret) >> goto out; >> + xenbus_probe(NULL); >> return 0; >> >> out: >> -- >> 1.7.0.4 >> >> -- >> Frank Pan >> >> Computer Science and Technology >> Tsinghua University > >> From 8d2402bd48ccc9eb5afe2aaa407d61ff715cb24c Mon Sep 17 00:00:00 2001 >> From: Frank Pan <frankpzh@gmail.com> >> Date: Thu, 3 Mar 2011 16:36:17 +0000 >> Subject: [PATCH] Fix a bug that shutdown watcher is disabled on pvops >> >> --- >> linux-2.6-xen/drivers/xen/platform-pci.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/linux-2.6-xen/drivers/xen/platform-pci.c b/linux-2.6-xen/drivers/xen/platform-pci.c >> index c01b5dd..f141cfe 100644 >> --- a/linux-2.6-xen/drivers/xen/platform-pci.c >> +++ b/linux-2.6-xen/drivers/xen/platform-pci.c >> @@ -162,10 +162,10 @@ static int __devinit platform_pci_init(struct pci_dev *pdev, >> ret = gnttab_init(); >> if (ret) >> goto out; >> - xenbus_probe(NULL); >> ret = xen_setup_shutdown_event(); >> if (ret) >> goto out; >> + xenbus_probe(NULL); >> return 0; >> >> out: >> -- >> 1.7.0.4 >> > >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > >-- 潘震皓, Frank Pan Computer Science and Technology Tsinghua University _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Mar-04 08:41 UTC
Re: [Xen-devel] [PATCH pvops] Fix a bug that shutdown watcher is disabled on pvops
On Fri, 2011-03-04 at 02:09 +0000, Frank Pan wrote:> Sorry I didn''t mention it. I use jeremy''s tree. > > git clone git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git > linux-2.6-xen > > The kernel version is 2.6.32.27.Is the issue you are seeing perhaps due to this patch being missing from that tree? commit a947f0f8f7012a5e8689a9cff7209ec6964ec154 Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Date: Mon Oct 4 16:10:06 2010 +0100 xen: do not set xenstored_ready before xenbus_probe on hvm Register_xenstore_notifier should guarantee that the caller gets notified even if xenstore is already up. Therefore we revert "do not notify callers from register_xenstore_notifier" and set xenstored_read at the right time for PV on HVM guests too. In fact in case of PV on HVM guests xenstored is ready only after the platform pci driver has completed the initialization, so do not set xenstored_ready before the call to xenbus_probe(). This patch fixes a shutdown_event watcher registration bug that causes "xm shutdown" not to work properly. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Acked-by: Jeremy Fitzhardinge <jeremy@goop.org> If possible we should prefer to backport patches from newer kernels rather than diverge in the 2.6.32 branch. Stefano''s "2.6.38-rc6-pvhvm" branch also contains this which looks related: commit 702d4eb9b3de4398ab99cf0a4e799e552c7ab756 Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Date: Thu Dec 2 17:54:50 2010 +0000 xen: no need to delay xen_setup_shutdown_event for hvm guests anymore Now that xenstore_ready is used correctly for PV on HVM guests too, we don''t need to delay the initialization of xen_setup_shutdown_event anymore. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Acked-by: Jeremy Fitzhardinge <jeremy@goop.org> Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Frank Pan
2011-Mar-04 09:29 UTC
Re: [Xen-devel] [PATCH pvops] Fix a bug that shutdown watcher is disabled on pvops
> Is the issue you are seeing perhaps due to this patch being missing from > that tree? > > commit a947f0f8f7012a5e8689a9cff7209ec6964ec154 > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Date: Mon Oct 4 16:10:06 2010 +0100 > > xen: do not set xenstored_ready before xenbus_probe on hvm> Stefano''s "2.6.38-rc6-pvhvm" branch also contains this which looks > related: > > commit 702d4eb9b3de4398ab99cf0a4e799e552c7ab756 > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Date: Thu Dec 2 17:54:50 2010 +0000 > > xen: no need to delay xen_setup_shutdown_event for hvm guests anymoreFP: Yes, both commits seem like a solution of the issue I met.> If possible we should prefer to backport patches from newer kernels > rather than diverge in the 2.6.32 branch.FP: The former commit seems ok. The latter one is more like a cleanup. Here is a patch can be applied on xen/next-2.6.32: --- linux-2.6-xen/drivers/xen/xenbus/xenbus_probe.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/linux-2.6-xen/drivers/xen/xenbus/xenbus_probe.c b/linux-2.6-xen/drivers/xen/xenbus/xenbus_probe.c index 3a83ba2..f368db9 100644 --- a/linux-2.6-xen/drivers/xen/xenbus/xenbus_probe.c +++ b/linux-2.6-xen/drivers/xen/xenbus/xenbus_probe.c @@ -646,7 +646,10 @@ int register_xenstore_notifier(struct notifier_block *nb) { int ret = 0; - blocking_notifier_chain_register(&xenstore_chain, nb); + if (xenstored_ready > 0) + ret = nb->notifier_call(nb, 0, NULL); + else + blocking_notifier_chain_register(&xenstore_chain, nb); return ret; } @@ -660,7 +663,7 @@ EXPORT_SYMBOL_GPL(unregister_xenstore_notifier); void xenbus_probe(struct work_struct *unused) { - BUG_ON((xenstored_ready <= 0)); + xenstored_ready = 1; /* Notify others that xenstore is up */ blocking_notifier_call_chain(&xenstore_chain, 0, NULL); @@ -722,7 +725,6 @@ static int __init xenbus_init(void) xen_store_interface = mfn_to_virt(xen_store_mfn); } else { - xenstored_ready = 1; if (xen_hvm_domain()) { uint64_t v = 0; err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v); @@ -738,6 +740,7 @@ static int __init xenbus_init(void) xen_store_evtchn = xen_start_info->store_evtchn; xen_store_mfn = xen_start_info->store_mfn; xen_store_interface = mfn_to_virt(xen_store_mfn); + xenstored_ready = 1; } } -- 1.7.0.4 -- 潘震皓, Frank Pan Computer Science and Technology Tsinghua University _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Mar-04 11:51 UTC
Re: [Xen-devel] [PATCH pvops] Fix a bug that shutdown watcher is disabled on pvops
On Fri, 2011-03-04 at 09:29 +0000, Frank Pan wrote:> > Is the issue you are seeing perhaps due to this patch being missing from > > that tree? > > > > commit a947f0f8f7012a5e8689a9cff7209ec6964ec154 > > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Date: Mon Oct 4 16:10:06 2010 +0100 > > > > xen: do not set xenstored_ready before xenbus_probe on hvm > > > Stefano''s "2.6.38-rc6-pvhvm" branch also contains this which looks > > related: > > > > commit 702d4eb9b3de4398ab99cf0a4e799e552c7ab756 > > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Date: Thu Dec 2 17:54:50 2010 +0000 > > > > xen: no need to delay xen_setup_shutdown_event for hvm guests anymore > > FP: Yes, both commits seem like a solution of the issue I met. > > > If possible we should prefer to backport patches from newer kernels > > rather than diverge in the 2.6.32 branch. > > FP: The former commit seems ok. The latter one is more like a cleanup. > Here is a patch can be applied on xen/next-2.6.32:Thanks, but we should just cherry pick the original. I actually pulled this patch into the Debian Squeeze (2.6.32.x) kernel a while ago and it hasn''t caused any problems TTBOMK. I kept forgetting to forward it on. Pull request below includes this path and a second one which I also pulled into the Debian kernel. Jeremy, please can you pull. Ian. The following changes since commit 892d2f052e979cf1916647c752b94cf62ec1c6dc: Jeremy Fitzhardinge (1): Merge commit ''v2.6.32.28'' into xen/next-2.6.32 are available in the git repository at: git://xenbits.xen.org/people/ianc/linux-2.6.git pvhvm-2.6.32 Stefano Stabellini (2): xen: do not set xenstored_ready before xenbus_probe on hvm xen: do not initialize PV timers on HVM if !xen_have_vector_callback arch/x86/xen/time.c | 5 +++-- drivers/xen/xenbus/xenbus_probe.c | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel