WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Tim Deegan <Tim.Deegan@xxxxxxxxxx>, Olaf Hering <olaf@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] tools: fix build after recent xenpaging changes
From: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Date: Fri, 24 Jun 2011 13:33:46 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 24 Jun 2011 05:35:06 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110624121613.GA17634@xxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
References: <20110624121613.GA17634@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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