git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Joachim Schmitz" <jojo@schmitz-digital.de>
To: <git@vger.kernel.org>, "'Paolo Bonzini'" <bonzini@gnu.org>
Cc: "'Junio C Hamano'" <gitster@pobox.com>,
	"'Erik Faye-Lund'" <kusmabite@gmail.com>, <bug-gnulib@gnu.org>,
	<rsbecker@nexbridge.com>
Subject: RE: poll() emulation in git
Date: Thu, 6 Sep 2012 16:02:30 +0200	[thread overview]
Message-ID: <010301cd8c38$4256bb90$c70432b0$@schmitz-digital.de> (raw)
In-Reply-To: <50476EFD.2000500@gnu.org>

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Wednesday, September 05, 2012 5:26 PM
> To: Joachim Schmitz
> Cc: 'Junio C Hamano'; git@vger.kernel.org; 'Erik Faye-Lund'; bug-gnulib@gnu.org
> Subject: Re: poll() emulation in git
> 
> Il 05/09/2012 15:36, Joachim Schmitz ha scritto:
> >>> > > Does your system have a working FIONREAD ioctl for pipes?
> >> >
> >> > It does have FIONREAD ioctl. Whether it works properly is to be determined...
> >> > I'll test if you could show me how?
> > Oh, now I see what you aimed at, but no, that Mac OS X method doesn't work for me, I tried (at least I think I did).
> >
> > And <sys/ioctl.h> has
> > /*
> >  * Normal IOCTL's supported by the socket interface
> >  */
> > #define FIONREAD        _IOR(0, 8, _ioctl_int)       /* Num of bytes to read */
> > #define FIONBIO         _IOW(0, 9, _ioctl_int)       /* Non-blocking I/O     */
> >
> > So these seem to be supported on sockets only, I guess.
> > And indeed the man pages for ioctl confirms:
> >
> >           Valid values for the request parameter for AF_INET or
> >           AF_INET6 sockets are:
> >
> >
> >           FIONREAD  Gets the number of bytes available for reading and
> >                     stores it at the int pointed at by arg.
> >
> >
> > So not even AF_UNIX sockets, not to mention pipes...
> 
> So there's no way you can support POLLHUP.  Your system is quite
> crippled. :(

Unfortunatly.

But is there something that could be done to make git work even without poll()?
It is used in 5 places:

$ grep -n poll\( *.c */*.c
credential-cache--daemon.c:175: if (poll(&pfd, 1, 1000 * wakeup) < 0) {
daemon.c:1018:          if (poll(pfd, socklist->nr, -1) < 0) {
help.c:361:                     poll(NULL, 0, autocorrect * 100);
upload-pack.c:232:              if (poll(pfd, pollsize, -1) < 0) {
builtin/upload-archive.c:125:           if (poll(pfd, 2, -1) < 0) {

Don't quite understand why in help.c it has that NULL, which should always result in an EFAULT and other than that basically is a
NOP (at least in the poll() emulation)? Seems a usleep(autocorrect * 100) is meant to happen here instead?
So I think here a poll() isn't needed at all. But also the 'broken' one shouldn't harm too much.

In daemon.c it seems to be all sockets it polls on, so it should work on NonStop.
Same in credential-cache--daemon.c

Remains upload-pack.c and builtin/upload-archive.c
In both start_command() gathers the FDs to poll() on and that indeed works on pipes -> problem on NonStop!

Seeing that in those cases xread() takes care of EAGAIN, I've now used 'brute force' in poll.c:

...
# else
      char data[64];
      r = recv (fd, data, sizeof (data), MSG_PEEK);
      socket_errno = (r < 0) ? errno : 0;
# endif
      if (r == 0)
        happened |= POLLHUP;

      /* If the event happened on an unconnected server socket,
         that's fine. */
      else if (r > 0 || ( /* (r == -1) && */ socket_errno == ENOTCONN))
        happened |= (POLLIN | POLLRDNORM) & sought;

      /* Distinguish hung-up sockets from other errors.  */
      else if (socket_errno == ESHUTDOWN || socket_errno == ECONNRESET
               || socket_errno == ECONNABORTED || socket_errno == ENETRESET)
        happened |= POLLHUP;

#ifdef __TANDEM /* as we can't recv(...,MSG_PEEK) on a non-socket */
      else if (socket_errno == ENOTSOCK)
        happened |= (POLLIN | POLLRDNORM) & sought;
#endif
      else
        happened |= POLLERR;
    }
...

We won't detect POLLHUP that way I think. However it seems to work, we've been able to clone, push, pull, branch that way with
NonStop being the (ssh-)server, something that didn't work at all without that hack (and yes, I believe it is just that).
Someone in for a cleaner way of managing this?

Bye, Jojo

  reply	other threads:[~2012-09-06 14:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-05 11:24 poll() emulation in git Joachim Schmitz
2012-09-05 11:55 ` Bastien ROUCARIES
2012-09-05 12:04   ` Joachim Schmitz
2012-09-05 12:05 ` Paolo Bonzini
2012-09-05 12:57   ` Joachim Schmitz
2012-09-05 13:36   ` Joachim Schmitz
2012-09-05 15:25     ` Paolo Bonzini
2012-09-06 14:02       ` Joachim Schmitz [this message]
2012-09-06 14:31         ` Paolo Bonzini
2012-09-06 14:44           ` Joachim Schmitz
2012-09-06 15:15             ` Paolo Bonzini
2012-09-07  7:23               ` Joachim Schmitz
2012-09-07  7:39           ` Joachim Schmitz
2012-09-07  9:40             ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='010301cd8c38$4256bb90$c70432b0$@schmitz-digital.de' \
    --to=jojo@schmitz-digital.de \
    --cc=bonzini@gnu.org \
    --cc=bug-gnulib@gnu.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kusmabite@gmail.com \
    --cc=rsbecker@nexbridge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).