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

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

 



Stefan Beller <sbeller@xxxxxxxxxx> writes:

> On Tue, Sep 22, 2015 at 8:58 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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 */
>     }

It is very likely that you will hit the same EAGAIN immediately by
continuing and end up spewing tons of the same warning, no?

We may want to slow down and think before sending a knee-jerk
response (this applies to both of us).

I actually think a blocking poll(2) here would not be such a bad
idea.  It would preserves the previous behaviour for callers who
unknowingly inherited a O_NONBLOCK file descriptor from its
environment without forcing them to waste CPU cycles.
--
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]