* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> > * Juan Quintela (quint...@redhat.com) wrote:
> >> We know quit closing the QIO.
> >
> > This patch does two different things; one of which I think I understand.
> >
> > The 'quit' has been removed - I think that makes sense because the
> > multifd threads terminate when either they come to the end of a stream
> > or hit an error; there's no case of asking them to quit explicitly.
> > The 'sem' was basically for kicking the recv-thread to quit; which again
> > isn't needed.
> >
> > Now, what about the object_unref on p->c ?
> > If I've got this right multifd_recv_terminate_threads is only called in
> > the error case; but doesn't multifd_load_cleanup also get called in that
> > case - it does the unref and p->c = NULL as well.
>
> Adding another comment O:-)
>
> In normal exit case: we don't care.
>
> In error case, we do a close of the p->c. Doing a close, means that the
> channel threads that are waiting on qio_channel_read_all_eof() will stop
> the waiting and return an error, so we are safe.
>
> ret = qio_channel_read_all_eof(p->c, (void *)p->packet,
> p->packet_len, &local_err);
> if (ret == 0) { /* EOF */
> break;
> }
> if (ret == -1) { /* Error */
> break;
> }
>
> We have to wait for three things:
> - we receive a synchronization packet
> - we receive EOF and finish
> - we got somehow an error and have to quit.
>
> Old versions have a semaphore, and we have three places where we set
> that semaphore "we are ready", when we receive one error, when we have
> data waiting to be read (it had a qio whatcher) or we just got a normal
> exit.
>
> Waiting on two things make code more complex that it used to be (and
> whatchers are really lame, because we know there are data ready, but not
> how much, dealing with non-whole packets/pages is a real mess). So we
> moved to:
> - quit is gone: we just close the channel, that makes the *_read_eof()
> to end. Notice that we don't care if the close is due to one error
> or because we have finished. It is just done.
> - Now reception channel only have to wait on _read_eof(), the three
> cases that we talked before are handled correctly.
>
> I understand the confusion, this only makes sense when you came from the
> previous version of the patchset.
Two questions from that then:
a) Are you sure it's safe to close the qio_channel while another
thread is in qio_channel_read_all_eof? Is it really defined that it
causes the other thread to exit with an error; close() in some stuff
frees data structures that the other thread is still reading; that's
why I've used shutdown(2) in the past rather than close on fd's
b) I don't think your answer explains why it's an object_unref?
Dave
> Later, Juan.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
