From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41537) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrQAw-00020l-Ce for qemu-devel@nongnu.org; Thu, 20 Nov 2014 06:45:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XrQAq-0000iF-7A for qemu-devel@nongnu.org; Thu, 20 Nov 2014 06:45:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37718) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrQAp-0000i2-U9 for qemu-devel@nongnu.org; Thu, 20 Nov 2014 06:45:20 -0500 Date: Thu, 20 Nov 2014 11:45:02 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20141120114502.GE5983@work-vm> References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-34-git-send-email-dgilbert@redhat.com> <54301FEE.10403@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54301FEE.10403@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 33/47] Postcopy: Postcopy startup in migration thread List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, quintela@redhat.com, cristian.klein@cs.umu.se, qemu-devel@nongnu.org, amit.shah@redhat.com, yanghy@cn.fujitsu.com * Paolo Bonzini (pbonzini@redhat.com) wrote: > Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto: > > From: "Dr. David Alan Gilbert" > > > > Rework the migration thread to setup and start postcopy. > > > > Signed-off-by: Dr. David Alan Gilbert > > --- > > include/migration/migration.h | 3 + > > migration.c | 201 ++++++++++++++++++++++++++++++++++++++---- > > 2 files changed, 185 insertions(+), 19 deletions(-) > > > > diff --git a/include/migration/migration.h b/include/migration/migration.h > > index b01cc17..f401775 100644 > > --- a/include/migration/migration.h > > +++ b/include/migration/migration.h > > @@ -125,6 +125,9 @@ struct MigrationState > > /* Flag set once the migration has been asked to enter postcopy */ > > volatile bool start_postcopy; > > > > + /* Flag set once the migration thread is running (and needs joining) */ > > + volatile bool started_migration_thread; > > volatile almost never does what you think it does. :) True. > In this case, I think only one thread reads/writes the variable so > "volatile" is unnecessary. Lets just check that; so it's set by 'migrate_fd_connect' (from the main thread) when it spawns the thread, and it's cleared by migrate_fd_cleanup that's always run as a bh, so should always be in the main thread; so yes - always the same thread, that's nice and simple; volatile evaporated. > Otherwise, you would need to add actual memory barriers, atomic > operations, or synchronization primitives. > > For start_postcopy, it is okay because it is just a hint to the compiler > and the processor will eventually see the assignment. Yes, in this case my understanding is that it's necessary to stop the compiler potentially moving the check outside the loop. > For this case > QEMU has atomic_read/atomic_set (corresponding to __ATOMIC_RELAXED in > C/C++1x), so you could use those as well. Ah, so those look like they just volatile cast anyway. (I've probably got some other flags I need to think about reading/writing atomically/safely). Dave (I'll take the other issues in this mail separately since there are quite a few). -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK