[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/14] qio: allow threaded qiotask to switch con
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH 13/14] qio: allow threaded qiotask to switch contexts |
Date: |
Thu, 1 Mar 2018 16:49:50 +0800 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Wed, Feb 28, 2018 at 01:20:24PM +0000, Daniel P. Berrangé wrote:
> 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é 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.
> > > >
> > > > We have done some work before on doing similar things to add non-default
> > > > gcontext support. The general idea is that we delete the old GSource
> > > > from the main context, then re-add a new one to the new context when
> > > > context changed to a non-default one. However this trick won't work
> > > > easily for threaded QIOTasks since we can't easily stop a real thread
> > > > and re-setup the whole thing from the very beginning.
> > >
> > > 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.
> >
> > Yeah I agree with you that the threaded QIO patches are complicated.
> > Then how about I introduce:
> >
> > - qio_task_thread_new(): to create QIO task and its thread (but not
> > running)
> > - qio_task_thread_run(): to run the thread
> >
> > Then I can setup the context correctly between the two in some way.
> >
> > 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.
I dropped patches 9-14 in version two and rewrote as you suggested, by
introducing a chardev machine done notifier. Please have a looks. Thanks,
--
Peter Xu
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 13/14] qio: allow threaded qiotask to switch contexts,
Peter Xu <=