Re: [PATCHv3 02/13] xread: poll on non blocking fds

From: Stefan Beller <sbeller@google.com>
Date: Sep 22, 2015, 17:38 UTC
Message-ID: <CAGZ79kaJmy4EijFuLCXOzPbi3EoqSwarFj3_3r+ghUwVA+88Mg@mail.gmail.com>
In-Reply-To: <xmqqh9mm5tus.fsf@gitster.mtv.corp.google.com>

On Tue, Sep 22, 2015 at 8:58 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 19 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>>>         while (1) {
>>>                 nr = read(fd, buf, len);
>>> -               if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
>>> -                       continue;
>>> +               if (nr < 0) {
>>> +                       if (errno == EINTR)
>>> +                               continue;
>>> +                       if (errno == EAGAIN || errno == EWOULDBLOCK) {
>>> +                               struct pollfd pfd;
>>> +                               int i;
>>> +                               pfd.events = POLLIN;
>>> +                               pfd.fd = fd;
>>> +                               i = poll(&pfd, 1, 100);
>>
>> Why is this poll() using a timeout? Isn't that still a busy wait of
>> sorts (even if less aggressive)?
>

True. Maybe we could have just a warning for now?

    if (errno == EAGAIN) {
        warning("Using xread with a non blocking fd");
        continue; /* preserve previous behavior */
    }

I think I am going to drop this patch off the main series and spin it out as an extra patch as the discussion is a bit unclear to me at the moment where we're heading.

Show 5 quoted lines

> Good point.  If we _were_ to have this kind of "hiding issues under
> the rug and continuing without issues" approach, I do not think we
> would need timeout for this poll(2).  The caller accepted that it is
> willing to wait until we read up to len (which is capped, though) by
> not calling the nonblocking variant.