Ian Campbell
2010-Aug-18 15:01 UTC
[Xen-devel] [PATCH] libxc: free thread specific hypercall buffer on xc_interface_close
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1282143629 -3600
# Node ID b1f4b4be1f94c0007794f86abd019f5c2629c59b
# Parent ddbd38da07397dca4760fb687551c3f6f9134700
libxc: free thread specific hypercall buffer on xc_interface_close
The per-thread hypercall buffer is usually cleaned up on pthread_exit
by the destructor passed to pthread_key_create. However if the calling
application is not threaded then the destructor is never called.
This frees the data for the current thread only but that is OK since
any other threads will be cleaned up by the destructor.
Changed since v1:
* Ensure hcall_buf_pkey is initialised before use. Thanks to
Christoph Egger for his help diagnosing this issue on NetBSD.
* Remove redundant if (hcall_buf) from xc_clean_hcall_buf since
_xc_clean_hcall_buf includes the same check.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r ddbd38da0739 -r b1f4b4be1f94 tools/libxc/xc_private.c
--- a/tools/libxc/xc_private.c Wed Aug 18 13:14:57 2010 +0100
+++ b/tools/libxc/xc_private.c Wed Aug 18 16:00:29 2010 +0100
@@ -57,6 +57,8 @@ xc_interface *xc_interface_open(xentooll
return 0;
}
+static void xc_clean_hcall_buf(void);
+
int xc_interface_close(xc_interface *xch)
{
int rc = 0;
@@ -68,6 +70,9 @@ int xc_interface_close(xc_interface *xch
rc = xc_interface_close_core(xch, xch->fd);
if (rc) PERROR("Could not close hypervisor interface");
}
+
+ xc_clean_hcall_buf();
+
free(xch);
return rc;
}
@@ -180,6 +185,8 @@ int hcall_buf_prep(void **addr, size_t l
int hcall_buf_prep(void **addr, size_t len) { return 0; }
void hcall_buf_release(void **addr, size_t len) { }
+static void xc_clean_hcall_buf(void) { }
+
#else /* !__sun__ */
int lock_pages(void *addr, size_t len)
@@ -228,6 +235,13 @@ static void _xc_init_hcall_buf(void)
static void _xc_init_hcall_buf(void)
{
pthread_key_create(&hcall_buf_pkey, _xc_clean_hcall_buf);
+}
+
+static void xc_clean_hcall_buf(void)
+{
+ pthread_once(&hcall_buf_pkey_once, _xc_init_hcall_buf);
+
+ _xc_clean_hcall_buf(pthread_getspecific(hcall_buf_pkey));
}
int hcall_buf_prep(void **addr, size_t len)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Christoph Egger
2010-Aug-18 16:04 UTC
Re: [Xen-devel] [PATCH] libxc: free thread specific hypercall buffer on xc_interface_close
This patch prevents me from starting a guest until the outstanding issue - namely why is hcall_buf_prep() never called - is solved. Christoph On Wednesday 18 August 2010 17:01:07 Ian Campbell wrote:> # HG changeset patch > # User Ian Campbell <ian.campbell@citrix.com> > # Date 1282143629 -3600 > # Node ID b1f4b4be1f94c0007794f86abd019f5c2629c59b > # Parent ddbd38da07397dca4760fb687551c3f6f9134700 > libxc: free thread specific hypercall buffer on xc_interface_close > > The per-thread hypercall buffer is usually cleaned up on pthread_exit > by the destructor passed to pthread_key_create. However if the calling > application is not threaded then the destructor is never called. > > This frees the data for the current thread only but that is OK since > any other threads will be cleaned up by the destructor. > > Changed since v1: > * Ensure hcall_buf_pkey is initialised before use. Thanks to > Christoph Egger for his help diagnosing this issue on NetBSD. > * Remove redundant if (hcall_buf) from xc_clean_hcall_buf since > _xc_clean_hcall_buf includes the same check. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > diff -r ddbd38da0739 -r b1f4b4be1f94 tools/libxc/xc_private.c > --- a/tools/libxc/xc_private.c Wed Aug 18 13:14:57 2010 +0100 > +++ b/tools/libxc/xc_private.c Wed Aug 18 16:00:29 2010 +0100 > @@ -57,6 +57,8 @@ xc_interface *xc_interface_open(xentooll > return 0; > } > > +static void xc_clean_hcall_buf(void); > + > int xc_interface_close(xc_interface *xch) > { > int rc = 0; > @@ -68,6 +70,9 @@ int xc_interface_close(xc_interface *xch > rc = xc_interface_close_core(xch, xch->fd); > if (rc) PERROR("Could not close hypervisor interface"); > } > + > + xc_clean_hcall_buf(); > + > free(xch); > return rc; > } > @@ -180,6 +185,8 @@ int hcall_buf_prep(void **addr, size_t l > int hcall_buf_prep(void **addr, size_t len) { return 0; } > void hcall_buf_release(void **addr, size_t len) { } > > +static void xc_clean_hcall_buf(void) { } > + > #else /* !__sun__ */ > > int lock_pages(void *addr, size_t len) > @@ -228,6 +235,13 @@ static void _xc_init_hcall_buf(void) > static void _xc_init_hcall_buf(void) > { > pthread_key_create(&hcall_buf_pkey, _xc_clean_hcall_buf); > +} > + > +static void xc_clean_hcall_buf(void) > +{ > + pthread_once(&hcall_buf_pkey_once, _xc_init_hcall_buf); > + > + _xc_clean_hcall_buf(pthread_getspecific(hcall_buf_pkey)); > } > > int hcall_buf_prep(void **addr, size_t len) > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Aug-18 16:26 UTC
Re: [Xen-devel] [PATCH] libxc: free thread specific hypercall buffer on xc_interface_close
On Wed, 2010-08-18 at 17:04 +0100, Christoph Egger wrote:> This patch prevents me from starting a guest until > the outstanding issue - namely why is hcall_buf_prep() never called - > is solved.You wouldn''t expect hcall_buf_prep to be called if you opened the xc interface and then closed it without doing a hypercall which required any locked down memory. This updated version of the patch was intended to handle exactly this case since it appears that under NetBSD it can cause issues. Are you saying that you are seeing hypercalls which you expect to need memory locking down but for which that has not happened? The callers of hcall_buf_prep are pretty explicit and there aren''t that many of them. With the attached patch to libxc I get the following output when starting xend, so you see it is quite normal for the xc interface to be called when hcall_buf_prep has never been called and we now handle that case correctly. xc_interface_open xc_interface_open xc_interface_open xc_interface_close xc_clean_hcall_buf _xc_init_hcall_buf _xc_clean_hcall_buf xc_interface_open xc_interface_open xc_interface_open xc_interface_open xc_interface_open xc_interface_open xc_interface_open xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf When starting a PV domain I get xc_interface_open xc_interface_close xc_clean_hcall_buf _xc_init_hcall_buf _xc_clean_hcall_buf Using config file "/etc/xen/debian-x86_32p-1". xc_interface_open hcall_buf_prep(0xbfb56ca8,144) _xc_init_hcall_buf hcall_buf_prep hcall_buf allocated at 0x804b0d8 hcall_buf_prep hcall_buf->buf allocated at 0x804c000 hcall_buf_prep using preallocated buffer at 0x804c000 for 0xbfb56ca8 hcall_buf_release(0x804c000,144) hcall_buf_release releasing preallocated buffer at 0x804c000 for 0xbfb56ca8 xc_interface_close xc_clean_hcall_buf _xc_clean_hcall_buf _xc_clean_hcall_buf unlock and free hcall_buf->buf 0x804c000 _xc_clean_hcall_buf free hcall_buf 0x804b0d8 It is also safe to call _xc_clean_hcall_buf and then later call hcall_buf_prep (and therefore allocate a buffer). Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2010-Aug-19 12:39 UTC
Re: [Xen-devel] [PATCH] libxc: free thread specific hypercall buffer on xc_interface_close
On Wednesday 18 August 2010 18:26:28 Ian Campbell wrote:> On Wed, 2010-08-18 at 17:04 +0100, Christoph Egger wrote: > > This patch prevents me from starting a guest until > > the outstanding issue - namely why is hcall_buf_prep() never called - > > is solved. > > You wouldn''t expect hcall_buf_prep to be called if you opened the xc > interface and then closed it without doing a hypercall which required > any locked down memory. > > This updated version of the patch was intended to handle exactly this > caseYes, it works. The issue was in the debug code itself. After removing the trace code I am able to start guests again. Thanks for your help.> since it appears that under NetBSD it can cause issues.I am wondering why this issue didn''t appear on Linux. Does Linux have some kind of auto-initialization? Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Aug-19 12:48 UTC
Re: [Xen-devel] [PATCH] libxc: free thread specific hypercall buffer on xc_interface_close
On Thu, 2010-08-19 at 13:39 +0100, Christoph Egger wrote:> On Wednesday 18 August 2010 18:26:28 Ian Campbell wrote: > > On Wed, 2010-08-18 at 17:04 +0100, Christoph Egger wrote: > > > This patch prevents me from starting a guest until > > > the outstanding issue - namely why is hcall_buf_prep() never called - > > > is solved. > > > > You wouldn''t expect hcall_buf_prep to be called if you opened the xc > > interface and then closed it without doing a hypercall which required > > any locked down memory. > > > > This updated version of the patch was intended to handle exactly this > > case > > Yes, it works. The issue was in the debug code itself. After > removing the trace code I am able to start guests again. > > Thanks for your help.Thanks for confirming.> > since it appears that under NetBSD it can cause issues. > > I am wondering why this issue didn''t appear on Linux. > Does Linux have some kind of auto-initialization?I think it''s just good/bad luck one way or the other if an uninitialised pthread_key_t looks valid to pthread_getspecific or not. Ultimately pthread_key_t is likely just an integer so there is always a chance that an uninitialised pthread_key_t looks like an existing valid key. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel