From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47096) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1er1f4-0003ss-EQ for qemu-devel@nongnu.org; Wed, 28 Feb 2018 08:20:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1er1f0-00044F-9Z for qemu-devel@nongnu.org; Wed, 28 Feb 2018 08:20:46 -0500 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47464 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1er1f0-000444-4U for qemu-devel@nongnu.org; Wed, 28 Feb 2018 08:20:42 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E6EA44040073 for ; Wed, 28 Feb 2018 13:20:37 +0000 (UTC) Date: Wed, 28 Feb 2018 13:20:24 +0000 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Message-ID: <20180228132024.GJ17774@redhat.com> Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= References: <20180228050633.7410-1-peterx@redhat.com> <20180228050633.7410-14-peterx@redhat.com> <20180228092356.GF31550@redhat.com> <20180228130526.GK27381@xz-mi> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20180228130526.GK27381@xz-mi> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 13/14] qio: allow threaded qiotask to switch contexts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: qemu-devel@nongnu.org, Paolo Bonzini , Juan Quintela , Markus Armbruster , =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , Stefan Hajnoczi , "Dr . David Alan Gilbert" On Wed, Feb 28, 2018 at 09:05:26PM +0800, Peter Xu wrote: > On Wed, Feb 28, 2018 at 09:23:56AM +0000, Daniel P. Berrang=C3=A9 wrote= : > > On Wed, Feb 28, 2018 at 01:06:32PM +0800, Peter Xu wrote: > > > This is the part of work to allow the QIOTask to use a different > > > gcontext rather than the default main gcontext, by providing > > > qio_task_context_set() API. > > >=20 > > > We have done some work before on doing similar things to add non-de= fault > > > gcontext support. The general idea is that we delete the old GSour= ce > > > from the main context, then re-add a new one to the new context whe= n > > > context changed to a non-default one. However this trick won't wor= k > > > easily for threaded QIOTasks since we can't easily stop a real thre= ad > > > and re-setup the whole thing from the very beginning. > >=20 > > I think this entire usage pattern is really broken. We should not > > provide a way to change the GMainContext on an existing task. We > > should always just set the correct GMainContxt right from the start. > > This will avoid much of the complexity you're introducing into this > > patch series, and avoid having to expose GTasks to the callers of > > the async methods at all. >=20 > Yeah I agree with you that the threaded QIO patches are complicated. > Then how about I introduce: >=20 > - qio_task_thread_new(): to create QIO task and its thread (but not > running) > - qio_task_thread_run(): to run the thread >=20 > Then I can setup the context correctly between the two in some way. >=20 > After that, qio_task_run_in_thread() will be two calls of above two > APIs. I don't see why it is not enough to just pass the right GMainContext into qio_task_run_in_thread() immediately. There should not be any need to split this into two parts. ALl that's needed here is for the completion callback to rnu in the right context, which just means passing the GMainContext from qio_task_run_in_thread, through to qio_task_thread_worker via QIOTaskThreadData. Changing the GMainContext after the thread is already created/running is not something we should try todo. Regards, Daniel --=20 |: https://berrange.com -o- https://www.flickr.com/photos/dberran= ge :| |: https://libvirt.org -o- https://fstop138.berrange.c= om :| |: https://entangle-photo.org -o- https://www.instagram.com/dberran= ge :|