[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] tools: fix build after recent xenpaging changes



On Fri, 2011-06-24 at 13:16 +0100, Tim Deegan wrote:
> tools: fix build after recent xenpaging changes
> xenpaging now uses pthreads, so must link appropriately.

Why does 23625:c49e22648d0e need a new thread to do the page in on exit?
Can't it just signal the main loop to do it?

Also page_in_trigger doesn't seem safe to me:
+void page_in_trigger(unsigned long gfn)
+{
+    if (!page_in_possible)
+        return;
+
+    pthread_mutex_lock(&page_in_mutex);
+    page_in_gfn = gfn;
+    pthread_mutex_unlock(&page_in_mutex);
+    pthread_cond_signal(&page_in_cond);
+}

Two back to back calls to this function (which is what the caller will
do) will both update page_in_gfn without the page in thread necessarily
running in the interim. i.e. the first gfn may be missed. I don't think
pthread_cond_signal makes any guarantees about whether this thread or
the signalled thread will run afterwards. For this approach to woek
page_in_gfn really needs to remain locked until the page in thread has
finished with that particular entry, or you need s return signal, or a
queue, or whatever.

I suppose you could also push the "/* Write all pages back into the
guest */" loop down into the thread rather than feeding the thread mfns
one-by-one.

Ian.

> 
> Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
> 
> diff -r 2633588c2427 tools/xenpaging/Makefile
> --- a/tools/xenpaging/Makefile        Fri Jun 24 13:03:38 2011 +0100
> +++ b/tools/xenpaging/Makefile        Fri Jun 24 13:10:34 2011 +0100
> @@ -2,7 +2,7 @@ XEN_ROOT=$(CURDIR)/../..
>  include $(XEN_ROOT)/tools/Rules.mk
>  
>  CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenstore)
> -LDLIBS += $(LDLIBS_libxenctrl) $(LDLIBS_libxenstore)
> +LDLIBS += $(LDLIBS_libxenctrl) $(LDLIBS_libxenstore) -pthread
>  
>  POLICY    = default
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.