From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38507) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dfM2s-0000hG-Rc for qemu-devel@nongnu.org; Wed, 09 Aug 2017 04:08:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dfM2n-00048h-TK for qemu-devel@nongnu.org; Wed, 09 Aug 2017 04:08:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49712) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dfM2n-00047u-Kn for qemu-devel@nongnu.org; Wed, 09 Aug 2017 04:08:45 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AE33AC0587C6 for ; Wed, 9 Aug 2017 08:08:44 +0000 (UTC) Date: Wed, 9 Aug 2017 16:08:38 +0800 From: Peter Xu Message-ID: <20170809080838.GK13486@pxdev.xzpeter.org> References: <20170717134238.1966-1-quintela@redhat.com> <20170717134238.1966-10-quintela@redhat.com> <20170720093455.GC23385@pxdev.xzpeter.org> <874ltibemw.fsf@secure.mitica> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <874ltibemw.fsf@secure.mitica> Subject: Re: [Qemu-devel] [PATCH v5 09/17] migration: Start of multiple fd work List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org, dgilbert@redhat.com, lvivier@redhat.com, berrange@redhat.com On Tue, Aug 08, 2017 at 11:19:35AM +0200, Juan Quintela wrote: > Peter Xu wrote: > > On Mon, Jul 17, 2017 at 03:42:30PM +0200, Juan Quintela wrote: > > > > [...] > > > >> int multifd_load_setup(void) > >> { > >> int thread_count; > >> - uint8_t i; > >> > >> if (!migrate_use_multifd()) { > >> return 0; > >> } > >> thread_count = migrate_multifd_threads(); > >> multifd_recv_state = g_malloc0(sizeof(*multifd_recv_state)); > >> - multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count); > >> + multifd_recv_state->params = g_new0(MultiFDRecvParams *, thread_count); > >> multifd_recv_state->count = 0; > >> - for (i = 0; i < thread_count; i++) { > >> - char thread_name[16]; > >> - MultiFDRecvParams *p = &multifd_recv_state->params[i]; > >> - > >> - qemu_mutex_init(&p->mutex); > >> - qemu_sem_init(&p->sem, 0); > >> - p->quit = false; > >> - p->id = i; > >> - snprintf(thread_name, sizeof(thread_name), "multifdrecv_%d", i); > >> - qemu_thread_create(&p->thread, thread_name, multifd_recv_thread, p, > >> - QEMU_THREAD_JOINABLE); > >> - multifd_recv_state->count++; > >> - } > > > > Could I ask why we explicitly switched from MultiFDRecvParams[] array > > into a pointer array? Can we still use the old array? Thanks, > > Now, we could receive the channels out of order (the wonders of > networking). So, we have two options that I can see: > > * Add interesting global locking to be able to modify inplace (I know > that it should be safe, but yet). > * Create a new struct in the new connection, and then atomically switch > the pointer to the right instruction. > > I can assure you that the second one makes it much more easier to detect > when you use the "channel" before you have fully created it O:-) Oh, so it's possible that we start to recv pages even if the recv channel has not yet been established... Then would current code be problematic? Like in multifd_recv_page() we have: static void multifd_recv_page(uint8_t *address, uint16_t fd_num) { ... p = multifd_recv_state->params[fd_num]; qemu_sem_wait(&p->ready); ... } Here can p==NULL if channel is not ready yet? (If so, I think a static array makes more sense...) Thanks, -- Peter Xu