Re: [PATCHv2 2/7] xread: poll on non blocking fds

["Torsten Bögershausen" <tboegi@web.de>]

On 17.12.15 21:22, Stefan Beller wrote:
> On Thu, Dec 17, 2015 at 12:12 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> On 16.12.15 01:04, Stefan Beller wrote:
>>> The man page of read(2) says:
>>>
>>>   EAGAIN The file descriptor fd refers to a file other than a socket
>>>        and has been marked nonblocking (O_NONBLOCK), and the read
>>>        would block.
>>>
>>>   EAGAIN or EWOULDBLOCK
>>>        The file descriptor fd refers to a socket and has been marked
>>>        nonblocking (O_NONBLOCK), and the read would block.  POSIX.1-2001
>>>        allows either error to be returned for this case, and does not
>>>        require these constants to have the same value, so a portable
>>>        application should check for both possibilities.
>>>
>>> If we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK.
>>> As the intent of xread is to read as much as possible either until the
>>> fd is EOF or an actual error occurs, we can ease the feeder of the fd
>>> by not spinning the whole time, but rather wait for it politely by not
>>> busy waiting.
>>>
>>> We should not care if the call to poll failed, as we're in an infinite
>>> loop and can only get out with the correct read().
>> I'm not sure if this is valid under all circumstances:
>> This is what "man poll" says under Linux:
>> []
>>  ENOMEM There was no space to allocate file descriptor tables.
>> []
>> And this is from Mac OS, ("BSD System Calls Manual")
>> ERRORS
>>      Poll() will fail if:
>>
>>      [EAGAIN]           Allocation of internal data structures fails.  A sub-
>>                         sequent request may succeed.
>> And this is opengroup:
>> http://pubs.opengroup.org/onlinepubs/9699919799//functions/poll.html:
>> [EAGAIN]
>>     The allocation of internal data structures failed but a subsequent request may succeed.
>>
>> read() may return EAGAIN, but poll() may fail to allocate memory, and fail.
>> Is it always guaranteed that the loop is terminated?
> 
> In case poll fails (assume a no op for it), the logic should not have
> changed by this patch?
> 
> Looking closely:
> 
>>>       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;
>>> +                             pfd.events = POLLIN;
>>> +                             pfd.fd = fd;
>>> +                             /*
>>> +                              * it is OK if this poll() failed; we
>>> +                              * want to leave this infinite loop
>>> +                              * only when read() returns with
>>> +                              * success, or an expected failure,
>>> +                              * which would be checked by the next
>>> +                              * call to read(2).
>>> +                              */
>>> +                             poll(&pfd, 1, -1);
> 
> Or do you mean to insert another continue in here?
I was thinking that we run into similar loop as before:
read() returns -1; errno = EAGAIN  /* No data to read */
poll() returns -1; errno = EAGAIN /* poll failed. If the fd was OK, the failure may be temporaly,
                                    as much as poll() can see.
                                    But most probably we run out ouf memory */

So the code would look like this:

   if (!poll(&pfd, 1, -1))
      return -1;


> 

>>> +                     }
>>> +             }
>>>               return nr;
>>>       }
>>>  }
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>