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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 17, 2015 at 12:12 PM, Torsten Bögershausen <tboegi@xxxxxx> 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?

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]