[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] iothread: delay the context release to fina
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] iothread: delay the context release to finalize |
Date: |
Mon, 25 Sep 2017 13:58:12 +0800 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Mon, 09/25 13:50, Peter Xu wrote:
> On Mon, Sep 25, 2017 at 01:30:02PM +0800, Fam Zheng wrote:
> > On Mon, 09/25 13:23, Peter Xu wrote:
> > > On Fri, Sep 22, 2017 at 09:09:22PM +0800, Fam Zheng wrote:
> > > > On Fri, 09/22 16:56, Peter Xu wrote:
> > > > > When gcontext is used with iothread, the context will be destroyed
> > > > > during iothread_stop(). That's not good since sometimes we would like
> > > > > to keep the resources until iothread is destroyed, but we may want to
> > > > > stop the thread before that point.
> > > >
> > > > Would be nice if you can also mention the glib bug that "required" this
> > > > in the
> > > > commit message.
> > >
> > > I can add it, but I am not sure it's very closely related (and I'm
> > > afraid that may confuse more people). Say, even without that bug, I
> > > would still think it not a good idea to free the context in the loop,
> > > especially considering that we have the finalize function there. Thanks,
> >
> > It's interesting to know if or not your future change will break without
> > this
> > patch, this is especially useful for backport.
>
> I haven't tried to run with iothread and without this patch, but I
> think it should fail, so this patch should be needed.
>
> The point is that we should not destroy the context before explicitly
> calling remove_fd_in_watch() if the context is running chardevs.
> Without this patch, this rule does not satisfy. And IIUC this rule
> comes from the glib bug.
>
> Anyway, I'll mention it in commit message to clarify.
OK, thanks for the explanations! My r-b still stands with the amended commit
log.
Fam
- Re: [Qemu-devel] [PATCH 1/3] iothread: provide helpers for internal use, (continued)
[Qemu-devel] [PATCH 2/3] iothread: export iothread_stop(), Peter Xu, 2017/09/22
[Qemu-devel] [PATCH 3/3] iothread: delay the context release to finalize, Peter Xu, 2017/09/22